JetBrains / svg-sprite-loader

Webpack loader for creating SVG sprites.
MIT License
2.01k stars 272 forks source link

Wrong webpack installation gets loaded in nested node project #461

Closed protango closed 3 years ago

protango commented 3 years ago

Do you want to request a feature, report a bug or ask a question? Report a bug

What is the current behavior? Consider this project structure:

If we invoke webpack on project_b, the plugin will require files from the webpack@4 installation located inside project_a. (Specifically: webpack/lib/RuleSet).

Why is this an issue? While this may sound like a corner case, this setup is caused by working within a yarn workspaces monorepo structure (which is not that uncommon these days). Yarn workspaces hoists certain dependencies to the top level, so it's important that apps prefer to use their local node_modules folder before looking up the tree.

Using the wrong version of webpack causes unexpected issues, particularly when using a complicated webpack config as in the case of vue projects.

What is the expected behavior? In the above scenario the plugin should not require any webpack files from project_a because a webpack installation exists within project_b, and their versions differ.

If the current behavior is a bug, please provide the steps to reproduce, at least part of webpack config with loader configuration and piece of your code. The best way is to create repo with minimal setup to demonstrate a problem (package.json, webpack config and your code). It you don't want to create a repository - create a gist with multiple files

Run the following commands:

git clone https://github.com/protango/svg-loader-issue-1.git
cd svg-loader-issue-1
npm install
cd project_b
npm install
npm run build

You should see output similar to the following: image

The test code injects the "Loaded webpack v4..." message into webpack/lib/RuleSet within the v4 webpack installation. The message should not show because the file should not be loaded, the plugin should use webpack v5 from the project_b node_modules.

If this is a feature request, what is motivation or use case for changing the behavior?

Please tell us about your environment:

Other information (e.g. detailed explanation, stacktraces, related issues, suggestions how to fix, links for us to have context, eg. stackoverflow, gitter, etc)

The issue exists here: https://github.com/JetBrains/svg-sprite-loader/blob/master/lib/utils/get-matched-rule.js

Because it tries to resolve webpack v4 first and this will succeed because webpack v4 does indeed exist but at a level higher than expected.

Related issues: https://github.com/JetBrains/svg-sprite-loader/issues/437 https://github.com/JetBrains/svg-sprite-loader/issues/417

protango commented 3 years ago

@d3x42 I'm trying to fix this myself, but realised I'd basically be reverting #440 (I was going to add a webpack version check to the get-matched-rule.js file). Can you help me out by explaining the rationale for #440? The get-webpack-version code looks like it should work fine, though #437 and #417 seem to indicate otherwise. Is there some edge case where this doesn't work?

d3x42 commented 3 years ago

@protango Please check 309 and 338

protango commented 3 years ago

Thanks @d3x42, I've put through another PR #463 that resolves this problem by forcing get-matched-rule-4 to only load webpack/lib/RuleSet from the nearest webpack installation (ie. the one resolved by require('webpack')). This ensures incorrect webpack installations will not be loaded without having to check the version number.

d3x42 commented 3 years ago

Seems to be fine, i hope it won't affect other users

d3x42 commented 3 years ago

svg-sprite-loader@6.0.9