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

0.13.5 broken #2859

Closed
Brianzchen opened this issue Aug 18, 2023 · 9 comments
Closed

0.13.5 broken #2859

Brianzchen opened this issue Aug 18, 2023 · 9 comments

Comments

@Brianzchen
Copy link

Hi @ljharb, I haven't dug into it but upgrading from 0.13.2 to latest has broken this plugin, I've found it come from 0.13.5 and the lodash revert from 0.13.6 hasn't seemed to have solved anything.

I'll be locking to 0.13.4 for now but hope it can be fixed soon!

@ljharb
Copy link
Member

ljharb commented Aug 18, 2023

To be clear, are you saying that #2857 is not solved for you in 0.13.6? What exactly are you seeing?

Can you check 0.13.4, and confirm if the problem is there?

@miguelrincon
Copy link

miguelrincon commented Aug 18, 2023

Hi @ljharb, I see Unable to resolve path to module errors when running eslint using eslint-import-resolver-webpack@0.13.6.

I debugged the array.prototype.find and lodash/get changes in 0.13.4..0.13.6 and I came to the conclusion that the issue is related to the update of resolve@^2.0.0-next.4.

I haven't yet gone deeper into the issue with resolve but it seems to be related to webpack aliases we use.

This is eslint run with the entire error log: https://gitlab.com/gitlab-renovate-forks/gitlab/-/jobs/4893864394

Our update merge request: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129630

@ljharb
Copy link
Member

ljharb commented Aug 18, 2023

@miguelrincon no version of resolve should have been able to resolve an image by default, and the webpack resolver doesn't pass any options into resolve that would enable it, so I'm not sure how that would be the case. However, you can npm install --no-save resolve@1 and see if that fixes it?

@andersk
Copy link
Contributor

andersk commented Aug 18, 2023

resolve@2.x adds browserify/resolve@698a3e1 “[Breaking] add isDirectory; error early if provided basedir is not a directory”. But we’re setting basedir to the webpack config file path, which is a file, not a directory.

basedir = cwd || configPath;

@ljharb
Copy link
Member

ljharb commented Aug 18, 2023

oh shit, good call. i guess previously it just silently ignored that base dir? If we path.dirname(basedir), will that fix the problem, or will it introduce a new problem?

I've got the fix ready, but i'll wait til i get a pulse check here :-)

@andersk
Copy link
Contributor

andersk commented Aug 18, 2023

Presumably you mean path.dirname(configPath), since cwd would be a directory already if it’s set?

That works for my project at least.

@Brianzchen
Copy link
Author

To be clear, are you saying that #2857 is not solved for you in 0.13.6? What exactly are you seeing?

Can you check 0.13.4, and confirm if the problem is there?

I was on v0.13.2 and tested every version until it broke so yep, 0.13.4 is not broken.

The problem I'm seeing is that it cannot resolve alias' I've defined in webpack.config.js. Where my eslint config looks like

module.exports = {
  settings: {
    'import/resolver': {
      webpack: {
        config: 'website/webpack.config.js',
      },
    },
  },
};

Presumably you mean path.dirname(configPath), since cwd would be a directory already if it’s set?

That works for my project at least.

On 0.13.6 and making that change in my node_modules for testing also fixes the problem for me :)

@miguelrincon
Copy link

@andersk you are right, passing an explicit cwd in .eslintrc works because that prevents the basedir passed to resolve from being configPath.

diff --git a/.eslintrc.yml b/.eslintrc.yml
index dfb9e8f97eb9..34ccbcbaa453 100644
--- a/.eslintrc.yml
+++ b/.eslintrc.yml
@@ -16,7 +16,8 @@ plugins:
 settings:
   import/resolver:
     webpack:
-      config: './config/webpack.config.js'
+      cwd: '.'
+      config: 'config/webpack.config.js'
 rules:
   import/no-commonjs: error
   import/no-default-export: off

@ljharb, I changed eslint-import-resolver-webpack/index.js locally and it works 👍 :

basedir = cwd || path.dirname(configPath);

@ljharb ljharb closed this as completed in 04e68a2 Aug 19, 2023
@ljharb
Copy link
Member

ljharb commented Aug 19, 2023

v0.13.7 published.

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

No branches or pull requests

4 participants