Skip to content
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

Enhanced TurboSnap: Trace dependency changes instead of bailing out #683

Merged
merged 29 commits into from
Dec 19, 2022

Conversation

ghengeveld
Copy link
Member

@ghengeveld ghengeveld commented Nov 25, 2022

Currently (as of v6.11.0), TurboSnap bails out if it finds a dependency-related change in package.json, or any change to package-lock.json or yarn.lock. Although this is much better than bailing out on any change to package.json, this behavior is still lacking because it means we do a full rebuild even if the updated dependency isn't used in any or some of the stories (directly or indirectly).

This update avoids bailing out of TurboSnap for changes to lock files. Instead it parses and compares lock files to their previous (baseline) version, to retrieve a set of changed dependency names. Those dependency names are then cross-referenced with the Webpack dependency tree to trace those changed dependencies to the story files that are affected by them. This essentially uses the existing "changed files" behavior of TurboSnap, but extends it to include files in node_modules (for dependencies we know have been added/updated/removed) in the set of changed files.

If we fail to find or parse a lock file (or its previous version) then we fall back to the v6.11.0 behavior of bailing out on any dependency-related change to package.json.

How to test

Scenarios to test:

  • Component change => tests are skipped as usual (except for the changed component)
  • npm dependency update (package-lock.json change) => only tests stories which use this dependency
  • Yarn dependency update (yarn.lock change) => only tests stories which use this dependency
  • package.json dependency change (lockfile missing) => bails out with changedPackageFiles
  • package.json other change (lockfile missing) => skips all tests
  • package.json dependency change (lockfile unchanged) => skips all tests
  • Monorepo setup (Storybook / package.json in subdirectory):
    • Component change => tests are skipped as usual (except for the changed component)
    • Dependency change in Storybook project (local lockfile change) => skips tests that don't use this dependency
    • Dependency change in another project (root lockfile change) => skips tests that don't use this dependency

Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome @ghengeveld! 🚀

I just want to make sure we get the replacement builds thing right. Is it possible to get a test for those in? I'm not really seeing a full E2E test for this, is that too difficult?

bin-src/tasks/gitInfo.ts Outdated Show resolved Hide resolved
bin-src/lib/findChangedDependencies.ts Outdated Show resolved Hide resolved
bin-src/tasks/upload.ts Outdated Show resolved Hide resolved
bin-src/git/git.ts Outdated Show resolved Hide resolved
Copy link
Member

@skitterm skitterm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, thanks for taking this on. I assume we'll want to also (not in this PR) change the TurboSnap docs related to full rebuilds.

bin-src/lib/findChangedPackageFiles.ts Show resolved Hide resolved
bin-src/lib/findChangedDependencies.ts Outdated Show resolved Hide resolved
bin-src/lib/findChangedDependencies.ts Outdated Show resolved Hide resolved
bin-src/tasks/upload.ts Outdated Show resolved Hide resolved
bin-src/lib/getDependentStoryFiles.test.ts Show resolved Hide resolved
@ghengeveld ghengeveld changed the title TurboSnap: Trace dependency changes instead of bailing out Enhanced TurboSnap: Trace dependency changes instead of bailing out Dec 12, 2022
Copy link
Member

@skitterm skitterm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me, thanks for all the comments in the tests!

@ghengeveld ghengeveld requested a review from tmeasday December 15, 2022 19:00
@ghengeveld
Copy link
Member Author

All tested ✅ Wow!
I used the shapes-light and chromatic projects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants