-
-
Notifications
You must be signed in to change notification settings - Fork 651
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
No longer able to exclude styles after upgrading from v12.0.4 to v12.0.5 #2847
Comments
I wonder if this relates at all to Vite's #4389, where CSS files are treated as side-effectual by default and aren't tree shaken correctly. The Vue Shepherd project uses Webpack instead of Vite for bundling its E2E tests and I just tested it: not including an explicit import does not include the default CSS rules. Might be worth spooling up a couple of skeleton projects for the two bundlers to clean room test it. If it's Vite-specific, it'll probably still need to be fixed (being one of the largest bundlers used), but at least that gives us an idea for why this suddenly happened. Though I couldn't say why Vite wouldn't have been doing this before with the wildcard, it may be possible to leverage that as a fix by using a wildcard for this again (like |
@Kenneth-Sills I think a wildcard might be a good fix here. Any way we can confirm easily or should we just try it? |
@RobbieTheWagner I'd always err on the side of caution and try to confirm things first, personally. 😅 So I created a totally fresh Vite project and took a look at what's going on. It's not all the styles being imported by default, it's just the styles for any Svelte component being added to the import graph (my demo only had the modal styles, for instance). That immediately excluded my prior guess that it was related to the Vite bug. So I took a closer look at our earlier PR and I think this comes down to the Rollup config changes we made. From the Rollup Svelte Plugin README:
Our first approach involved creating a separate build for the CSS bundle and disabling that "./dist/css/*": {
"import": "./dist/esm/css/*",
"require": "./dist/cjs/css/*"
}, Wildcard being used above, since that's just what I had tested with. If we were so inclined, we could also throw in a copy operation to the ESM build to output an extra to I've tested the above in my Vite demo project and it seems to work! I haven't had a chance to test it with a Webpack build though, nor have I run the E2E tests for this project against those changes. It's getting quite late, so that's something I'd have to tackle tomorrow. Anyway, let me know if you'd like to pick this up. If you're busy, I'd be happy to look at this again tomorrow night and try to get the behavior confirmed and a PR submitted before I'm out of commission Thursday. Thanks! |
@Kenneth-Sills we really appreciate this work and would absolutely welcome a PR if you're able. It would be great to do another release ASAP to help folks who don't want these extra styles and restore previous workflows. |
@chuckcarpenter You got it! PR submitted. |
Thanks for the quick response to this issue, great effort @Kenneth-Sills 👍🏽 |
* fix: reconfigure CSS bundling I've also added an E2E test group that gives us some basic monitoring for regressions in the CSS inclusion behavior. Closes #2847 * build: only distribute one copy of the css bundle We still generate all three copies, we just delete the extra ones at the end of the build process. * Reduce the number of builds --------- Co-authored-by: Robbie Wagner <[email protected]>
Hi all,
After upgrading from v12.0.4 to v12.0.5, the default shepherd css styles are now included even without the use of:
import 'shepherd.js/dist/css/shepherd.css';
Unfortunately for our usecase where we were not looking to use the shepherd css stylesheet and instead using our own css, the recent change has caused our tour styles to break due to the high specificity of the svelte component css rules.
expected behaviour
I can choose whether or not I want to use the default shepherd styles by including the
shepherd.css
stylesheet.actual behaviour
All shepherdjs svelte component styles are added after using
import Shepherd from 'shepherd.js'
shepherdjs svelte component styles included in head
Related discussion on issue #2834.
The text was updated successfully, but these errors were encountered: