birdofpreyru / babel-plugin-react-css-modules

Transforms styleName to className using compile time CSS module resolution.
https://dr.pogodin.studio/docs/babel-plugin-react-css-modules
Other
30 stars 12 forks source link

Doesn't work with yarn berry PnP mode #41

Open dobesv opened 1 year ago

dobesv commented 1 year ago

I tried migrating to this but I get an error when it tries to resolve the path to a css file:

 @dr.pogodin/babel-plugin-react-css-modules tried to access @draft-js-plugins/alignment, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.

Required package: @draft-js-plugins/alignment (via "@draft-js-plugins/alignment/lib/plugin.css")
Required by: @dr.pogodin/babel-plugin-react-css-modules@virtual:978be09f033f9d1d87a6ae470cf41768305eae1692596bdc022dee76fc3671ae951ebf07cf433ace646b0e1c79a9e1b2be00e15f7afac12075ecdcf897b30d3a#npm:6.9.4 (via /home/dobes/projects/formative/app/.yarn/__virtual__/@dr.pogodin-babel-plugin-react-css-modules-virtual-cbe7ecd98b/0/cache/@dr.pogodin-babel-plugin-react-css-modules-npm-6.9.4-458b93ef69-ad14c19a11.zip/node_modules/@dr.pogodin/babel-plugin-react-css-modules/dist/)

In Yarn PnP environments you cannot use require directly to access the dependencies of the packages that use your package. You have to do something like:

import { createRequire } from 'node:module';

// Sometime later

function resolveUsingImportLogic(path, projectDir = process.cwd()) {
  const targetRequire = createRequire(
    path.resolve(projectDir, 'package.json'),
  );
  const resolvedPath = targetRequire.resolve(thingYouWantToResolve);
}
birdofpreyru commented 1 year ago

Well... your issue seems to be somewhat beyond my NodeJS experience — I use NPM rather than Yarn; it is the first time I hear about the Yarn PnP environment; and in my head dynamic require() of a module is legit if the module can be found by the standard NodeJS resolution rules.

Thus, not surprised it does not work. I'll need to find time to educate myself about it, and to figure out what should be done about it. Also, at the first glance to your code snippet, my first doubt is that I imagine many projects may have package.json not located in the process.cwd() folder, so doing something like this probably would be a huge breaking change for existing projects relying on the normal NodeJS / NPM environment.

dobesv commented 1 year ago

I imagine many projects may have package.json not located in the process.cwd() folder

Sorry, I provided a bad example there.

The path passed to createRequire is the path of a file that is supposedly doing the import and is used to run the standard NodeJS resolution rules from that location instead of from the file that you are currently in (in the plugin). This is probably a better example:

import { createRequire } from 'node:module';

const getTargetResourcePath = (importedPath: string, stats: *) => {
  const targetFileDirectoryPath = dirname(stats.file.opts.filename);

  if (importedPath.startsWith('.')) {
    return resolve(targetFileDirectoryPath, importedPath);
  }
  const targetRequire = createRequire(stats.file.opts.filename);
  return targetRequire.resolve(importedPath);
};

Yarn Plug'n'Play adds a number of improvements to the node modules resolution system for better reliability & performance. You can read more here: https://yarnpkg.com/features/pnp

However, it does apply stricter rules when importing things - modules are not allowed to import things that are not declared in their own closest package.json. So when using require.resolve you have to use the above technique so Yarn recognizes which package the import is coming from and can apply the rule as approporiate for that package instead (the one being processed using babel, rather than the plugin code).

Note that even if yarn pnp is not being used, using createRequire to resolve relative to the specified module should be better in general as it will resolve the module path they would have gotten from that location.

Consider this somewhat contrived file tree:

node_modules
node_modules/babel
node_modules/@dr.pogodin/babel-plugin-react-css-modules
packages/a/node_modules/great-styles/a.css
packages/a/src/source-code.tsx

if packages/a/src/source-code.tsx imports great-styles/a.css your plugin won't find it because it would be looking for node modules up the wrong branch of the file tree.

birdofpreyru commented 1 year ago

For the record, I'll copy paste here a relevant note from the release v6.11.0.

Injecting an absolute path to a module into a compiled code is a terrible idea, as it implies that the compiled code will not work if its runtime environment is different from the build environment (e.g. modules have been moved around after the build, or the code is build on one system and used on another, etc.). I had a glance at pnpm documentation, and it seems to me that to avoid the problem PR #42 intended to solve, one just have to correctly configure his pnpm project, maybe to add some pnpm-specific configuration to this plugin as well. Not something I personally will spend my time on investigating further, unless somebody brings in funds to cover it. If you can't figure it out, neither able to bring in the funding for me to figure it out, I just suggest using the normal npm — it might be slower and less disk-space efficient, but it turns out it comes out with an easier developer experience :)