A race condition issue in postcss-custom-properties

[malash]

Bug description

Recently I found a race condition issue of postcss-custom-properties while using it with other async PostCSS plugins.

I cost a lot of time to figure out the root issue and finally I can provide a easy way to reproduce this bug.

Full example repo: https://github.com/malash/postcss-plugins-issue-331

To reproduce this issue you need:

1. Use postcss-custom-properties with another async plugin ( which means the listeners of plugin should return a promise ).

I implemented this fake plugin:

// this fake async plugin do nothing but await for next tick
const fakeAsyncPlugin = (opts = {}) => {
  // Plugin creator to check options or prepare caches
  return {
    postcssPlugin: 'A fake async plugin',
    Declaration: async () => {
      await new Promise(resolve => setTimeout(resolve, 0));
    },
  };
};
fakeAsyncPlugin.postcss = true;

2. Create a PostCSS instance and make sure the async runs before postcss-custom-properties.

const postcssInstance = postcss([
  fakeAsyncPlugin(),
  postcssCustomProperties({
    preserve: false,
  }),
]);

3. Call process method in parallel. Make sure one CSS contains custom properties and others doesn't.

const WITH_CUSTOM_PROPERTIES = `
:root {
  --color: red;
}

h1 {
  color: var(--color);
}
`;

const WITHOUT_CUSTOM_PROPERTIES = `
div {
  height: 10px;
}
`;

await Promise.all([
  postcssInstance.process(WITH_CUSTOM_PROPERTIES);
  postcssInstance.process(WITHOUT_CUSTOM_PROPERTIES);
  postcssInstance.process(WITHOUT_CUSTOM_PROPERTIES);
  postcssInstance.process(WITHOUT_CUSTOM_PROPERTIES);
  ...
]);

4. Check the output of WITH_CUSTOM_PROPERTIES, the var(--color) is not replaced as expected.

In my case I use PostCSS with Webpack, the postcss-loader usually run in parallel, that's why I fall into this bug.

Root issue and solution

I believe the root cause is this line:

postcss-plugins/plugins/postcss-custom-properties/src/index.ts

Line 51 in e9893a0

let customProperties: Map<string, valuesParser.ParsedValue> = new Map();

The plugin function is initialed only once in the postcss([pluginFn()]) call. But the prepare listener runs for each CSS input file. In this plugin, the customProperties should be placed `prepare function.

// let customProperties: Map<string, valuesParser.ParsedValue> = new Map();

return {
  postcssPlugin: 'postcss-custom-properties',
  prepare() {
    // move to here
    let customProperties: Map<string, valuesParser.ParsedValue> = new Map();
    return {
      Once:() => {},
      Declaration: () => {},
    };
  },
};

Source CSS

:root {
  --color: red;
}

h1 {
  color: var(--color);
}

Expected CSS

h1 {
  color: red;
}

Actual CSS

:root {
  --color: red;
}

h1 {
  color: var(--color);
}

Does it happen with npx @csstools/csstools-cli <plugin-name> minimal-example.css?

No

Debug output

No response

Extra config

No response

What plugin are you experiencing this issue on?

No response

Plugin version

12.1.6

What OS are you experiencing this on?

macOS

Node Version

16.13.0

Validations

• Follow our Code of Conduct
• Check that there isn't already an issue that request the same feature to avoid creating a duplicate.

Would you like to open a PR for this bug?

• I'm willing to open a PR

[romainmenke]

Thank you for reporting this!

I don't think you even need the second async plugin to trigger this.
The plugin isn't safe to use in this way :/

At the moment it only supports reuse of the same instance in series, not in parallel.
We will have to take a deep dive to see if this can be improved.

To use the plugin in parallel you can create multiple instances.

[malash]

@romainmenke I added a Root issue and solution section in this issue's description. I tested it in my project and it works.

Could you take a look? Also if it is ok I'm glad to create a PR later.

[romainmenke]

My current gut feeling is that everything should be moved into prepare.
But I am unsure of what the side effects of this will be.

Any PR is always welcome!

[malash]

@romainmenke #332 😄

[Antonio-Laguna]

@malash this has been released as 12.1.7. Thanks a lot for reporting and fixing this!