Throw an error when mixing extract: true and classHashPrefix configuration options to avoid unsupported usage and bundle size bloat.
#1724
Conversation
…iguration options to avoid unsupported usage and bundle size bloat.
kylorhall-atlassian
requested review from
JakeLane,
pancaspe87,
liamqma and
dddlr
as code owners
🦋 Changeset detectedLatest commit: d8e7ec3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 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 |
✅ Deploy Preview for compiled-css-in-js canceled.
|
| * Avoid mixing this with extraction as this may throw an error if combined with extraction | ||
| * or `extract: true` in Webpack loaders or Parcel tranformers. |
There was a problem hiding this comment.
This documentation isn't relevant, I don't think you can mix Babel + extraction elsewhere, but just covering my bases…
| // @ts-expect-error -- Make sure this config doesn't bleed in as it's passed through | ||
| if (options.classHashPrefix) { | ||
| throw new Error( | ||
| '`@compiled/webpack-loader.CompiledExtractPlugin` is mixing `extract: true` and `classHashPrefix` options, which will not supported and will result in bundle size bloat.' | ||
| ); | ||
| } |
There was a problem hiding this comment.
I've removed the config actually, so you can't do CompiledExtractPlugin({ classHashPrefix: … }), but kept the @ts-expect-error to document it in full (because it'd work without TS blocking you, I believe.
| @@ -211,7 +211,6 @@ module.exports = { | |||
| `@compiled/webpack-loader` also accepts the following options. These are not used in the loader itself, but instead they are passed directly to the underlying `@compiled/babel-plugin`. | |||
|
|
|||
| - `addComponentName` | |||
| - `classHashPrefix` | |||
There was a problem hiding this comment.
This was actually on the wrong place, this says it works with @compiled/webpack-loader, but I'm pretty sure it only works with new CompiledExtractPlugin({…}). Could be wrong!
| @@ -124,6 +124,12 @@ export default new Transformer<ParcelTransformerOpts>({ | |||
| }, | |||
|
|
|||
| async transform({ asset, config, options }) { | |||
| if (config.extract && config.classHashPrefix) { | |||
There was a problem hiding this comment.
note that config.extract is currently ignored in non-production environments when using Parcel
(stylesheet extraction doesn't work in non-prod environments when using Parcel)
There was a problem hiding this comment.
Got it, this kind of might be the wrong thing in that case if there's no different configs…I'm not sure exactly, might wait to see the PR from @guilhermehto to see if this would blow up or not.
Co-authored-by: Grant Wong <[email protected]>
…://github.com/atlassian-labs/compiled into kylorhall/throw-error-extract-classHashPrefix
What is this change?
Following #1717, throws an error when Parcel attempts to transform or Webpack's
CompiledExtractPluginis initialized with bothextract: trueandclassHashPrefixbeing defined.Why are we making this change?
To avoid unsupported usage and bundle size creep as adding 5 bytes to every class is not something we want to generally promote and instead
classHashPrefixis only desired as a temporary workaround to avoid conflicts in scenarios where non-extraction is used.How are we making this change?
Into the Parcel Transformer and Webpack Loader, the entrypoints where this configuration bleeds in. Technically this wasn't documented properly for
@compiled/webpack-loader, so I adjusted it briefly, but kept thethrow new Errorin place to avoid this (as it would still be passed through, I'd gather).PR checklist
updated or added applicable tests— I don't think this is feasible without making a brand new fixture for each scenario.website/— Modified briefly, but I'm not sure it's worth documenting in full (this already isn't documented in full).