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

Throw an error when mixing extract: true and classHashPrefix configuration options to avoid unsupported usage and bundle size bloat. #1724

Merged
Merged
7 changes: 7 additions & 0 deletions .changeset/fluffy-cherries-exercise.md
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
5 changes: 5 additions & 0 deletions .changeset/seven-beers-shake.md
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
6 changes: 6 additions & 0 deletions .changeset/silver-jokes-care.md
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.
3 changes: 3 additions & 0 deletions packages/babel-plugin/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ export interface PluginOptions {
/**
* Adds a defined prefix to the generated classes' hashes.
* Useful in micro frontend environments to avoid clashing/specificity issues.
*
* Avoid mixing this with extraction as this may throw an error if combined with extraction
* or `extract: true` in Webpack loaders or Parcel tranformers.
Comment on lines +111 to +112
Copy link
Collaborator Author

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…

*/
classHashPrefix?: string;
}
Expand Down
6 changes: 6 additions & 0 deletions packages/parcel-transformer/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,12 @@ export default new Transformer<ParcelTransformerOpts>({
},

async transform({ asset, config, options }) {
if (config.extract && config.classHashPrefix) {
Copy link
Collaborator

@dddlr dddlr Oct 15, 2024

Choose a reason for hiding this comment

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

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)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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)) {
Expand Down
2 changes: 2 additions & 0 deletions packages/parcel-transformer/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ export interface ParcelTransformerOpts extends BabelPluginOpts {
/**
* Adds a defined prefix to the generated classes' hashes.
* Useful in micro frontend environments to avoid clashing/specificity issues.
*
* This will throw an error if used alongside extraction or `extract: true`.
*/
classHashPrefix?: string;
}
5 changes: 5 additions & 0 deletions packages/webpack-loader/src/compiled-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ function getLoaderOptions(context: LoaderContext<CompiledLoaderOptions>) {
extractStylesToDirectory = undefined,
resolver = undefined,
importSources = undefined,
classHashPrefix = undefined,
}: CompiledLoaderOptions = typeof context.getOptions === 'undefined'
? // Webpack v4 flow
getOptions(context)
Expand Down Expand Up @@ -95,6 +96,9 @@ function getLoaderOptions(context: LoaderContext<CompiledLoaderOptions>) {
importSources: {
type: 'array',
},
classHashPrefix: {
type: 'string',
},
},
});

Expand All @@ -115,6 +119,7 @@ function getLoaderOptions(context: LoaderContext<CompiledLoaderOptions>) {
extractStylesToDirectory,
resolver,
importSources,
classHashPrefix,
};
}

Expand Down
7 changes: 7 additions & 0 deletions packages/webpack-loader/src/extract-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This may not work given the classHashPrefix is passed to the babel plugin isn't it? I'm a bit uncertain how this works to be frank!

}

apply(compiler: Compiler): void {
Expand Down
15 changes: 9 additions & 6 deletions packages/webpack-loader/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,15 @@ export interface CompiledLoaderOptions {
* Passed to @compiled/babel-plugin.
*/
importSources?: string[];

/**
* Adds a defined prefix to the generated classes' hashes.
* Useful in micro frontend environments to avoid clashing/specificity issues.
*
* Avoid mixing this with extraction as this may throw an error if combined with extraction
* or `extract: true` in Webpack loaders or Parcel tranformers.
*/
classHashPrefix?: string;
}

export interface CompiledExtractPluginOptions {
Expand Down Expand Up @@ -142,10 +151,4 @@ export interface CompiledExtractPluginOptions {
* Defaults to `false`.
*/
sortShorthand?: boolean;

/**
* Adds a defined prefix to the generated classes' hashes.
* Useful in micro frontend environments to avoid clashing/specificity issues.
*/
classHashPrefix?: string;
}
2 changes: 1 addition & 1 deletion website/packages/docs/src/pages/pkg-babel-plugin.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ Will cache the result of statically evaluated imports.

Adds a prefix to the generated hashed css rule names. The valued passed to it gets hashed in conjunction with the rest of the rule declaration.

This is useful when `@compiled` is being used in a micro frontend environment by multiple packages and you want to avoid specificity issues.
This is useful when `@compiled` is being used in a micro frontend environment by multiple packages and you want to avoid specificity issues. Please note that mixing this with extraction is not supported and Parcel or Webpack will throw an error if combined.

The currently accepted regex for this value is `^[a-zA-Z\-_]+[a-zA-Z\-_0-9]*$`.

Expand Down
Loading