WIP: Dependency finder for smarter builds #52471
Conversation
matticbot
added
[Status] Needs Review
|
This PR does not affect the size of JS and CSS bundles shipped to the user's browser. Generated by performance advisor bot at iscalypsofastyet.com. |
| 'packages/wpcom-proxy-request', | ||
| 'packages/wpcom.js', | ||
| ].map( ( pkg ) => path.resolve( pkg ) ); | ||
| function getMonorepoPackages() { |
There was a problem hiding this comment.
Maybe use ./build-tools/lib/monorepo.js
There was a problem hiding this comment.
Perfect! I'm not sure how to include it, though. I keep getting the error Could not find a declaration file for module '../../../build-tools/lib/monorepo'. when I add import { packagesInMonorepo } from '../../../build-tools/lib/monorepo';
There was a problem hiding this comment.
I experimented with an index.d.ts file (declaring a module for monorepo), but it didn't seem to help
|
|
||
| // Add the first variation which exists to the accumulator. | ||
| for ( const possibleFile of fileNameVariations ) { | ||
| const packagePath = resolve( 'node_modules', dirName, possibleFile ); |
There was a problem hiding this comment.
This would need to keep walking the tree structure up for more node_modules (eg: ./client/node_modules, then ./node_modules, then ../node_modules..., right?
Maybe use webpack's https://github.com/webpack/enhanced-resolve instead of building our own.
Or even directly use sass-loader? Looks like all their importing logic is exported in https://github.com/webpack-contrib/sass-loader/blob/master/src/utils.js
There was a problem hiding this comment.
Possibly, yeah. This approach does work for all imports in ETK, but it's likely missing stuff. Will look into those libraries!
noahtallen
force-pushed
the
try/dependency-finder
branch
from
f6ce5d1 to
a7c144f
Compare
|
Alright, this is in a pretty solid state. I've been working through all the edges just parsing through one package's files (ETK), and it's performing well.
The only missing file is actually an entry related to the webpack dependency extraction plugin. The unknown files right now are mostly a combination of the following:
I'm not exactly sure how to tie those into the results. 🤔 |
Eventually we would need to make the tool smarter: it should traverse node_modules dependencies, so if a transitive dependency changes it knows it has to re-build ETK. For now maybe we can just assume that if
The idea is that if any of those files can alter the compilation result (maybe |
Hmm, yeah. At this point, most of the "unknown" files can change the compilation result, but they are not compiled by webpack. I'm thinking of them as static assets. The bin scripts are a mixed bag, some of them can alter, some can't. I'm wondering what the most flexible approach would be. I think it would be hard to maintain a list of all files which can alter the build but are not processed by webpack. (We'd have to pick through the bin scripts of every app/package.) Is it reasonable to assume they can all affect the result, unless we know it's safe? For example, we could exclude all .gitignore and eslint config files. But we could assume that modifications to a
Yeah.. I was thinking that we could do something like that following:
Do we need to inspect the files further than just knowing that we have a dependency, and then trigger the build for $foo in two scenarios?
|
noahtallen
force-pushed
the
try/dependency-finder
branch
from
949c625 to
a3b61dc
Compare
That's a good point, we can do that. We won't need mosts of the dependency walking code, we can just define when to build ETK based on
Yes! We should apply the same logic that we apply for
Yep, but we need to remember to do it for any package in the full dependency tree, not only the direct dependencies. Probably the best way to generate that tree is look at |
One of my worries is having to create this lists for every single thing we want to process. I could see that being hard to maintain, especially if we want to expand this tool across the organization 🤔
true! I guess the main problem with using just "include" with no dependency traversal would be if something in ETK imported something from elsewhere in the monorepo without declaring it in package.json. And I guess that probably isn't a huge problem either as we enforce dependency lint rules more strictly. |
To correctly parse JS dependencies, we need to: 1. Pass a long enough directory path so that enhanced-resolve can find the root node_modules 2. Pass the webpack config
noahtallen
force-pushed
the
try/dependency-finder
branch
from
a3b61dc to
362bf2e
Compare
|
Found a way to read the current changed files into our script: https://teamcity.a8c.com/buildConfiguration/calypso_SmartBuildLauncher/6241702?buildTab=log&expandAll=true&focusLine=264 |
|
Here's the suggestion from the sass dependency finder tool for how to find node_modules: dependents/node-dependency-tree#132 (comment) |
|
For next steps, I was exploring the connection between TeamCity and the changed files. Essentially, the TeamCity build calls the script in dependency finder. The dependency finder reads a list of changed files from TeamCity. It also reads a list of different projects which include TeamCity project IDs. (For now, I put that in package-map.json. There might be a better spot.) The next step is comparing the changed files to the list of jobs, and then calling the TeamCity API to trigger that Job ID. I started writing a "find matching jobs" function, but I don't think it quite works yet. (https://github.com/Automattic/wp-calypso/pull/52471/files#diff-b71cfeff41c95e0ccfbf40d4f246e9b798c762d0644362cc9b3573529e3f1a64R104-R114) Everything should be pretty well typed, though. |
|
We probably won't be moving forward with this. |
github-actions
bot
removed
the
[Status] Needs Review
Changes proposed in this Pull Request
This is the initial implementation of the dependency finder package. The ultimate goal is to be able to take a certain CI/CD build x, and then determine when it should run. Builds should run when files which would change the build output are modified.
This package attempts to find all files which could possibly affect the build output of a given package. Here are some items it finds:
It also lists files which don't fit into the above definitions:
To Do
Testing instructions
Run this in your shell to test just for ETK. Leave out the apps/editing-toolkit portion if you want to see for all packages/apps.
You should see all the files it detects in the different categories.