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

oxc-resolver has been a breaking change #242

Closed
mrginglymus opened this issue Mar 15, 2025 · 16 comments · Fixed by #243 or #245
Closed

oxc-resolver has been a breaking change #242

mrginglymus opened this issue Mar 15, 2025 · 16 comments · Fixed by #243 or #245

Comments

@mrginglymus
Copy link

I've just tried to update to latest version of the plugin. oxc-resolver appears incompatible with yarn PnP (at least out of the box).

Switching to the node-modules linker at least allows it to resolve packages, but it then seems to have some trouble importing .css files which previously worked without additional config.

{
    settings: {
      "import-x/resolver-next": [
        importPlugin.createNodeResolver({
          extensions: [".ts", ".tsx", ".js"],
          conditionNames: ["my-condition", "import"],
          extensionAlias: {
            ".js": [".ts", ".js"],
          },
        }),
      ],
      "import-x/internal-regex": "^@my-namespace/",
      "import-x/external-module-folders": [".yarn"],
      "import-x/parsers": {
        "@typescript-eslint/parser": [".ts", ".tsx"],
      },
    },
@JounQin
Copy link
Member

JounQin commented Mar 15, 2025

Can you provide a runnable reproduction?

@mrginglymus
Copy link
Author

mrginglymus commented Mar 15, 2025

oxc-project/oxc-resolver#53 looks like pnp support is not yet enabled in oxc-resolver?

edit; or possibly not. will try and make a minimal repro

@JounQin
Copy link
Member

JounQin commented Mar 15, 2025

web-infra-dev/rspack-resolver#53 could be replacement, I'll publish into npm manually for now.

@mrginglymus
Copy link
Author

https://github.com/mrginglymus/epix-oxc trivial repro - works fine with node-modules, explodes with pnp

I suspect the css import issue might be fixable with config.

@JounQin
Copy link
Member

JounQin commented Mar 15, 2025

@mrginglymus Could you help to test #243?

# yarn 1
yarn add https://pkg.csb.dev/un-ts/eslint-plugin-import-x/commit/d794a858/eslint-plugin-import-x
# yarn 2, 3
yarn add eslint-plugin-import-x@https://pkg.csb.dev/un-ts/eslint-plugin-import-x/commit/d794a858/eslint-plugin-import-x/_pkg.tgz
# npm
npm i https://pkg.csb.dev/un-ts/eslint-plugin-import-x/commit/d794a858/eslint-plugin-import-x

@mrginglymus
Copy link
Author

This works a lot better! It's now able to resolve all modules. I am, however, still hitting two issues:

  1. I have some custom export conditions which resolve to a path with a trailing querystring. This is probably a vile abuse, but it used to work and now doesn't.
  2. rspack-resolver does not resolve pnpapi

@mrginglymus
Copy link
Author

mrginglymus commented Mar 15, 2025

For 1. I can fix with config using an extension alias:

{
  extensionAlias: {
    ".css?parp": [".css"]
  }
}

though that then does result in import-x claiming 'duplicate' imports where there are none.

@mrginglymus
Copy link
Author

I've updated the repro using #243 and added a pnpapi import.

@JounQin
Copy link
Member

JounQin commented Mar 15, 2025

I have some custom export conditions which resolve to a path with a trailing querystring. This is probably a vile abuse, but it used to work and now doesn't.

@mrginglymus Can you add this into the reproduction?

For pnpapi:

# yarn 1
yarn add https://pkg.csb.dev/un-ts/eslint-plugin-import-x/commit/c115e722/eslint-plugin-import-x
# yarn 2, 3
yarn add eslint-plugin-import-x@https://pkg.csb.dev/un-ts/eslint-plugin-import-x/commit/c115e722/eslint-plugin-import-x/_pkg.tgz
# npm
npm i https://pkg.csb.dev/un-ts/eslint-plugin-import-x/commit/c115e722/eslint-plugin-import-x

@mrginglymus
Copy link
Author

@JounQin updated, thanks! pnpapi working, added repro for querystring import

@JounQin
Copy link
Member

JounQin commented Mar 15, 2025

I tried original oxc-resolver, it's not supported w/o pnp, but I think it should be supported. cc @Boshen

{
  "exports": {
    ".": "./index.js",
    "./add": "./add.js?custom"
  }
}

I'll try to fix it at our own fork temporarily.

@JounQin
Copy link
Member

JounQin commented Mar 15, 2025

# yarn 1
yarn add https://pkg.csb.dev/un-ts/eslint-plugin-import-x/commit/27012bc4/eslint-plugin-import-x
# yarn 2, 3
yarn add eslint-plugin-import-x@https://pkg.csb.dev/un-ts/eslint-plugin-import-x/commit/27012bc4/eslint-plugin-import-x/_pkg.tgz
# npm
npm i https://pkg.csb.dev/un-ts/eslint-plugin-import-x/commit/27012bc4/eslint-plugin-import-x

@mrginglymus Working as expected now.

Image

@mrginglymus
Copy link
Author

Amazing! Yes, works for me. The only remaining difference is that now pnpapi is coming up as a different import group - though I think this new behaviour is more correct as it's getting grouped as an external dep.

Thanks so much for the quick turnaround on this.

@JounQin
Copy link
Member

JounQin commented Mar 15, 2025

The only remaining difference is that now pnpapi is coming up as a different import group - though I think this new behaviour is more correct as it's getting grouped as an external dep.

@mrginglymus In my PR https://github.com/un-ts/eslint-plugin-import-x/pull/243/files#diff-ad71c478de212fc2376c95a6ef7a5b2b24f2e2791c15b1d1c362d6342e4d0ae4R54, I marked pnpapi as core module for pnp, but I just checked enhanced-resolve, it resolves pnpapi as <root>/.pnp.cjs, maybe I should align its behavior.

@JounQin JounQin reopened this Mar 15, 2025
@JounQin
Copy link
Member

JounQin commented Mar 15, 2025

Should we mark pnpapi as core in PnP? This is a question.

@mrginglymus @SukkaW What do you think?

@mrginglymus
Copy link
Author

@JounQin I've updated to the latest version and I'm getting intermittent linting failures of this plugin, which is a new one to me! It seems to be a import-x/no-unresolved for every third party import. Will keep investigating and see if I can find a cause...

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