-
Notifications
You must be signed in to change notification settings - Fork 108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Proposal] Dependency Files as Inputs With Symlinked Packages #944
Comments
If this is something that would be a good fit for Wireit I wouldn't mind taking a crack at implementing it! |
So am I understanding correctly? You have package A, which depends on package B. Package A uses wireit, but package B does not. You want A to re-run whenever B changes. I think the first solution I would reach for in this situation is to just include the relevant files of B in the "files" array for A. To keep things organized, you could create a "files-only" target and use that as a dependency: {
"name": "a",
"wireit": {
"build": {
"files": [
"lib/**/*.js"
],
"dependencies": [
"b:files"
]
},
"b:files": {
"files": [
"../b/lib/**/*.js"
]
}
}
} Would something like that be a good solution for your use case? |
The case of a package using wireit while another does not was just another one that I noticed when I was writing this proposal. In my specific use-case I have a number of packages with this kind of config: {
"scripts": {
"build": "pnpm --filter=\"...$npm_package_name\" run build:project",
"build:project": "wireit"
},
"wireit": {
"build:project": {
"command": "tsc --project tsconfig.json",
"files": [
"tsconfig.json",
"src/**/*.{js,jsx,ts,tsx}",
"typings/**/*.ts"
],
"output": [
"dist/**"
]
}
}
} When I run
This is an interesting solution and I think it would work if also paired with one for #23 that automatically reads workspace dependencies. Assuming that each dependency had a wireit-only |
For the use-case of a non-wireit dependency, however, you'd also need to define all of the transitive dependencies too and honestly it sounds like a bit of a mess. Maintaining lists of dependencies by hand separately from the typical NPM dependencies seems a little error-prone and likely to result in accidental drift. |
One thing I do like about this proposal though is that it feels more intuitive. Package lock files are included in the fingerprint because we are acknowledging that changes in dependencies should invalidate the cache. Symlinked dependencies aren't included in that fingerprint but should also invalidate the cache. Since only the output files have any bearing on the current package, including those in the fingerprint just seems natural. |
I implemented this in our monorepo using a "wireit": {
"dependencyOutputs": {
"allowUsuallyExcludedPaths": true,
"files": [
"node_modules/@woocommerce/eslint-plugin/configs",
"node_modules/@woocommerce/eslint-plugin/rules",
"node_modules/@woocommerce/eslint-plugin/index.js",
"package.json"
]
}
} It's definitely not as pretty as a native solution in Wireit could be but It does feel intuitive when paired with a different solution for task orchestration. We can use Wireit alongside PNPM's |
Problem
Wireit does not provide a way to easily include the files of symlinked dependencies in fingerprinting. Unlike normal dependencies, changes to these will not be reflected in a lock file and will not change the fingerprint. This can be somewhat addressed by using the
"dependencies"
feature in Wireit, however, this is not always the case:wireit
in isolation in each package and rely on another solution for executing dependency and transitive dependency scripts. These won't define"dependencies"
inwireit
since that would result in multiple executions of the same scripts.wireit
don't define files to fingerprint or dependencies to include transitively.'devDependencies'
may not have a script to depend on. This can (probably?) be solved using a Wireit-only script that does nothing but define files and dependencies, but that feels like a hack.Proposed Solution
Provide a new
dependencyFiles
option that treats the files included in dependencies and transitive dependencies as fingerprinting inputs. By default this should includedependencies
,devDependencies
, andpeerDependencies
since changes to any of these might result in changes to outputs. This option should have these settings:true
: Enables the feature.exclude-dev
: Enables the feature and ignoresdevDependencies
.false
: Disables the feature (default).Performance Considerations
While it would be easy to simply include all files, in the case of a symlink, there will be many files that we have no business treating as inputs. There are some steps we can take to fingerprint the smallest number of files possible:
file:
dependencies as well as patterns used by workspaces in currently supported package managers.Benefits
Ultimately the goal here is to provide a way for
wireit
to be used with an individual package in isolation. By treating dependency files as inputs we can rely onwireit
to handle caching and watching for a package regardless of whether or not its dependencies are using it too.In my particular use-case, I am using
pnpm
and want to rely on--filter
to provide a consistent syntax for developers to run commands in our monorepo. Using Wireit's"dependencies"
means having various side-effects when using--filter
on scripts that use it. Including dependency outputs as package inputs means that each one can be built independently and we can rely on our own tooling to handle hierarchical tasks.This doesn't seem incompatible with the existing
"dependencies"
functionality either since downstream file changes would happen prior to the generation of a new fingerprint upstream. It will also play nicely with--watch
since downstream changes would automatically cascade.Alternatives Considered
Add
node_modules
to"files"
You can technically already get something kind of like this by pairing the
"allowUsuallyExcludedPaths"
setting with explicitnode_modules
globs. The caveat here is that you'll need to be very explicit in order to minimize unnecessary inputs. By relying onpackage.json
files to read dependencies we will avoid the headaches and pitfalls associated with manually maintaining a list like this. You can avoid these pitfalls with a custom tool but I believe this feature makes more sense to implement directly in Wireit.The text was updated successfully, but these errors were encountered: