-
Notifications
You must be signed in to change notification settings - Fork 68
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
Throw an error when mixing extract: true
and classHashPrefix
configuration options to avoid unsupported usage and bundle size bloat.
#1724
Changes from all commits
29a65ed
4acfa57
bc9a409
e06da63
b356876
d8e7ec3
5c19185
bbab282
1db036d
035de35
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
--- | ||
'@compiled/parcel-transformer': patch | ||
'@compiled/webpack-loader': patch | ||
'@compiled/babel-plugin': patch | ||
--- | ||
|
||
Documents what happens when mixing extraction and classHashPrefix |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@compiled/webpack-loader': patch | ||
--- | ||
|
||
fix: webpack loader wasn't passing classHashPrefix option down to babel plugin |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
--- | ||
'@compiled/parcel-transformer': minor | ||
'@compiled/webpack-loader': minor | ||
--- | ||
|
||
Throw an error when mixing `extract: true` and `classHashPrefix` configuration options to avoid unsupported usage and bundle size bloat. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note that (stylesheet extraction doesn't work in non-prod environments when using Parcel) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
throw new Error( | ||
'`@compiled/parcel-transformer` is mixing `extract: true` and `classHashPrefix` options, which will not supported and will result in bundle size bloat.' | ||
); | ||
} | ||
|
||
const ast = await asset.getAST(); | ||
|
||
if (!(ast?.type === 'babel' && ast.program)) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -108,6 +108,13 @@ export class CompiledExtractPlugin { | |
|
||
constructor(options: CompiledExtractPluginOptions = {}) { | ||
this.#options = options; | ||
|
||
// @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 the `extract: true` and `classHashPrefix` options, which is not supported and will result in bundle size bloat.' | ||
); | ||
} | ||
Comment on lines
+111
to
+117
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This may not work given the |
||
} | ||
|
||
apply(compiler: Compiler): void { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This documentation isn't relevant, I don't think you can mix Babel + extraction elsewhere, but just covering my bases…