fix: add *.css to sideEffects in all CSS-exporting graphiql packages#4211
fix: add *.css to sideEffects in all CSS-exporting graphiql packages#4211dimaMachina merged 3 commits intographql:mainfrom
*.css to sideEffects in all CSS-exporting graphiql packages#4211Conversation
🦋 Changeset detectedLatest commit: adcf505 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
trevor-scheer
left a comment
There was a problem hiding this comment.
This looks great and thanks for the fantastic PR description. Going to validate the change on my side when I get a chance but LGTM!
| "@graphiql/plugin-code-exporter": minor | ||
| "@graphiql/plugin-doc-explorer": minor | ||
| "@graphiql/plugin-explorer": minor | ||
| "@graphiql/plugin-history": minor | ||
| "@graphiql/react": minor | ||
| "graphiql": minor |
There was a problem hiding this comment.
Seems like a patch/bugfix to me, @dimaMachina ?
There was a problem hiding this comment.
I've just updated these patch (and also formatted it to pass CI); I'd intended this originally but the yarn changeset CLI puzzled me on first use.
Anyway, having CSS output where it's intended to exist but isn't currently working feels like a fix, but please let me know if this needs further adjusting.
dimaMachina
left a comment
There was a problem hiding this comment.
Lgtm, strange that nobody reported it before, looks like it's very strict in webpack
When attempting to set up
graphiqlin my Webpack 5 environment, and follow the example code https://github.com/graphql/graphiql/tree/main/packages/graphiql#using-as-package, I noticed that the line:was having no effect. The line was being processed, but Webpack was not emitting CSS. The resulting page looked like this, a working UI, but no CSS styles:
The cause is that the
graphiqlpackage has asideEffectsproperty in itspackage.json, which is set up to allow side effects but only fordist/setup-workers/*. Becausegraphiql/style.css(akadist/style.css) falls outside of this path, Webpack ignores the imported CSS.This is explained at https://webpack.js.org/guides/tree-shaking/#mark-the-file-as-side-effect-free, down the bottom of this section:
This PR follows that directive and adds the required
sideEffectsentry to all packages - especiallygraphiql- that have CSS exports in theirpackage.jsonfiles. Testing locally by patching this in to my build pipeline sees the JSimportline now working.Ahead of this PR being accepted, a Webpack configuration can manually override
sideEffectslike so:That forces
sideEffectsto be on for all files, but in Webpack, it's only able to be a Boolean.Another workaround is to use Sass/SCSS or another CSS preprocessor and
@use 'graphiql/style.css';instead -- using another tool bypasses Webpack's checking ofsideEffects. This is actually how I found the issue in the first place -- importing within Sass worked fine, but when usingimportin JS, nothing loaded.