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

Ensure options are passed to plugins when using postcss.config.js #170

Merged
merged 6 commits into from
Jan 12, 2024

Conversation

JohnAlbin
Copy link
Contributor

@JohnAlbin JohnAlbin commented Mar 26, 2021

If a postcss.config.js is used (instead of passing a plugins array), the PostCSS options passed to gulp-postcss are ignored by all PostCSS plugins.

If you look at the default export for postcs-load-config, you'll see:

declare function postcssrc(ctx?: ConfigContext, path?: string, options?: CosmiconfigOptions): Promise<Result>;
export = postcssrc;

where ConfigContext is defined as:

import { ProcessOptions } from "postcss";

// The full shape of the ConfigContext.
type ConfigContext = Context & ProcessOptionsPreload & RemainingProcessOptions;

// Additional context options that postcss-load-config understands.
interface Context {
    cwd?: string;
    env?: string;
}

interface ProcessOptionsPreload {
    parser?: string | ProcessOptions['parser'];
    stringifier?: string | ProcessOptions['stringifier'];
    syntax?: string | ProcessOptions['syntax'];
}

// The remaining ProcessOptions, sans the three above.
type RemainingProcessOptions =
    Pick<ProcessOptions, Exclude<keyof ProcessOptions, keyof ProcessOptionsPreload>>;

and ProcessOptions, defined in PostCSS, contains these keys: from, to, parser, stringifier, syntax, map.

TLDR; the first argument to postcssrc() is a single object with Context and PostCSS' ProcessOptions merged together.

But gulp-postcss is calling postcssrc() with { file: file, options: contextOptions } as its first argument. I noticed 2 things with this:

file is not a part of Context or ProcessOptions. But I see that postcss-load-config's README says:

Most third-party integrations add additional properties (emphasis mine) to the ctx (e.g postcss-loader).

It looks like that is what gulp-postcss is doing with the additional file property. HOWEVER, the ProcessOptions are not passed as a part of the ctx properties; instead they are put into a sub-property, ctx.options. That means the options passed to gulp-postcss are ignored by all PostCSS plugins since they are not where they are supposed to be.

@coveralls
Copy link

coveralls commented Mar 26, 2021

Coverage Status

Coverage remained the same at 100.0% when pulling 5b98eae on JohnAlbin:pass-options-to-plugins into a1b8242 on postcss:master.

@JohnAlbin
Copy link
Contributor Author

JohnAlbin commented Mar 26, 2021

I discovered this while using postcss-url and passing { to: 'outputfile.css' } to gulpPostCSS() was ignored by the plugin.

@JohnAlbin
Copy link
Contributor Author

JohnAlbin commented Mar 26, 2021

I consider passing the options in as ctx.options a bug, but I can see that this behaviour is explicitly documented in the code snippet at the bottom of gulp-postcss's README:

module.exports = function (ctx) {
    var file = ctx.file;
    var options = ctx.options;

I've fixed the bug and the above code snippet, but I've also added 1cd7e21 to ensure backwards compatibility with 9.0.0 if you want to release a 9.0.1 after this bugifx. That commit should be reverted before a 10.0.0 release is made, IMO.

@JohnAlbin JohnAlbin changed the title Ensure options are passed to plugins Ensure options are passed to plugins when using postcss.config.js Mar 26, 2021
@w0rm w0rm merged commit c21fca9 into postcss:main Jan 12, 2024
@w0rm
Copy link
Member

w0rm commented Jan 12, 2024

Thanks, I guess this is backwards compatible, so I am going to just merge this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants