Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add tests for --untraced flag validation #565

Merged

Conversation

ethriel3695
Copy link
Contributor

@ethriel3695 ethriel3695 commented May 10, 2022

Index

TS - TurboSnap
docs - documentation
PR - pull request
Global decorator - Story wrapper component in preview.js

Context

I've had a few customers reach out saying that TurboSnap bails out because of the preview.js-generated-config-entry.js file and variations of the file .jsx, .tsx

This is happening because of the dependencies being changed within a global decorator in the preview.js file

For instance, we have the following changed file:
src/packages/design-system/components/button.jsx // changed
Which then traces to this file change, which acts as a dynamic import of many components
src/packages/design-system/component/src/component.jsx
This traces to the file
.storybook/decorator.jsx
The decorator file is imported in preview.js as a global decorator and acts as a wrapper for each story

Really, the button is the only file being changed and all other traced changes are expected but not the global decorator because it's functionally created only for use as a wrapper for the story

We would like to advise customers to add the decorator to the flag --untraced **/decorator.jsx` like so

Explanation

These unit tests act as both a validation measure to verify that this functionality does not stop working and also as a sanity check to make sure that customers can realistically use this functionality and we can prove that it works correctly and as intended

I've got specific support examples and trace dependency outputs for internal use if we need more specific context but I do not want to share that here in the PR

Test cases

Four use cases are being handled with these tests when passing files to the flag --untraced:

  • Verify that TS ignores untraced files and dependencies with a glob pattern match ('/stories/')
  • Verify that TS does not bail when we pass global files ('**/(package**.json|yarn.lock)')
  • Verify that TS does not bail on untraced Storybook config files that change due to a module sibling ('**/decorator.jsx')
  • Verify that TS does not bail on a changed dependency from a dynamic import in an untraced config file ('**/decorator.jsx')

@linear
Copy link

linear bot commented May 10, 2022

CH-1867 Change --untraced flag to ignore the file being untraced and all imported components if the untraced file is a global decorator in preview.js

The BBC is currently experiencing an issue where they have a doc-decorator in preview.js which causes a ton of false-positive CSF dependency changes with TurboSnap

The --untraced flag doesn’t help (yet) because as currently implemented it only has an effect if the flagged file is the one that was changed (rather than the changes being in one of the dependencies of the flagged file).

This would allow customers to ignore global decorators and their dependencies so TurboSnap runs as expected

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.

Some questions/comments

@ethriel3695 ethriel3695 requested a review from tmeasday September 2, 2022 03:37
@ghengeveld ghengeveld merged commit 6501d61 into main Sep 2, 2022
@ghengeveld ghengeveld deleted the reuben/ch-1867-add-tests-for-untraced-flag-validation branch September 2, 2022 12:47
@ghengeveld ghengeveld changed the title Reuben/ch 1867 add tests for untraced flag validation Add tests for untraced flag validation Sep 2, 2022
@ghengeveld ghengeveld changed the title Add tests for untraced flag validation Add tests for --untraced flag validation Sep 2, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants