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

Check for rewritten pseudo states at rule-level rather than sheet-level #87

Conversation

ghengeveld
Copy link
Member

@ghengeveld ghengeveld commented Sep 1, 2023

This should allow sheets to be rewritten multiple times in case new rules are added later, whilst still not re-applying to already rewritten rules.

📦 Published PR as canary version: 2.1.2--canary.87.9ed8103.0

✨ Test out this PR locally via:

npm install storybook-addon-pseudo-states@2.1.2--canary.87.9ed8103.0
# or 
yarn add storybook-addon-pseudo-states@2.1.2--canary.87.9ed8103.0

@linear
Copy link

linear bot commented Sep 1, 2023

AP-3505 Pseudo states addon doesn't handle dynamic stylesheets

How is the user affected? And what is the expected behavior?

BBC ran into this issue. I had a meeting with Natalia earlier today we finally boiled it down to this:

#86

How many and/or what class of users does this impact?

Users of CSS frameworks that dynamically inject new styles into existing stylesheets (e.g. Emotion/Styled Components) in conjunction with an asynchronous process that triggers such behavior (e.g. theme switcher). Both are commonplace.

Is there a workaround?

Very much dependent on the implementation details. For BBC they can reload the page, but Chromatic snapshots are still broken, so it's not really a workaround.

What are the steps for reproducing the issue?

Stories render correctly when visiting with a URL that contains globals, but doesn't when visiting with a plain URL that doesn't. This is why it works locally and when they copy/paste the URL to share with coworkers, but doesn't work when navigating to a story from Chromatic, or when Chromatic takes screenshots. To reproduce, simply remove the globals from the URL before requesting the page.

Other information

The problem is with the __pseudoStatesRewritten: true which is a performance enhancement to prevent the pseudo states addon from rewriting the same stylesheet multiple times. When a stylesheet gets new style rules added to it dynamically, these style rules don't get rewritten. Most likely this can be fixed by changing the addon to add __pseudoStatesRewritten to every rule rather than the stylesheet as a whole.

Copy link
Member

@thafryer thafryer left a comment

Choose a reason for hiding this comment

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

LGTM

@ghengeveld ghengeveld merged commit 2d5328b into main Oct 11, 2023
6 checks passed
@ghengeveld ghengeveld deleted the ghengeveld/ap-3505-pseudo-states-addon-doesnt-handle-dynamic-stylesheets branch October 11, 2023 14:51
@github-actions
Copy link

🚀 PR was released in v2.1.2 🚀

@github-actions github-actions bot added the released This issue/pull request has been released. label Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants