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

Error while loading rule 'import/no-cycle': Package subpath './adapters/next/app' is not defined by "exports" #691

Closed
GBvanDam opened this issue Oct 22, 2024 · 21 comments · Fixed by #701
Labels
bug Something isn't working released

Comments

@GBvanDam
Copy link

GBvanDam commented Oct 22, 2024

Context

What's your version of nuqs?

"nuqs": "^2.0.0",

What framework are you using?

  • ✅ Next.js (app router)

Which version of your framework are you using?

"next": "^14.2.15",

Description

eslint fails on import { NuqsAdapter } from 'nuqs/adapters/next/app' with message:

Error while loading rule 'import/no-cycle': Package subpath './adapters/next/app' is not defined by "exports"

Reproduction

  1. import { NuqsAdapter } from 'nuqs/adapters/next/app'
  2. install "eslint": "^8.57.0"
  3. install "eslint-plugin-import": "^2.31.0"
  4. install "eslint-import-resolver-alias": "^1.1.2"
  5. run eslint
@GBvanDam GBvanDam added the bug Something isn't working label Oct 22, 2024
@franky47
Copy link
Member

franky47 commented Oct 22, 2024

Thanks for the report, the bundling looks and works fine on my end. What's your ESLint version?

Could you paste your tsconfig.json here please?

@GBvanDam
Copy link
Author

GBvanDam commented Oct 22, 2024

"eslint": "^8.57.0", no tsconfig in use, but I can see that indeed nuqs seems fine in node_modules. This one might be important:

"eslint-plugin-import": "^2.31.0",
"eslint-import-resolver-alias": "^1.1.2",

removing nuqs and adding again did not solve the issue. If it's not a bug in nuqs, no problem to close this issue, you don't have to fix my local issues, but I was worried that more users would face this whilst updating to the latest

jsconfig.json

{
  "compilerOptions": {
    "baseUrl": ".",
    "paths": {
      "@/assets/*": ["src/assets/*"],
      "@/auth/*": ["src/auth/*"],
      "@/components/*": ["src/components/*"],
      "@/hooks/*": ["src/hooks/*"],
      "@/layouts/*": ["src/layouts/*"],
      "@/locales/*": ["src/locales/*"],
      "@/handlers/*": ["src/handlers/*"],
      "@/routes/*": ["src/routes/*"],
      "@/sections/*": ["src/sections/*"],
      "@/theme/*": ["src/theme/*"],
      "@/utils/*": ["src/utils/*"]
    }
  }
}

@franky47
Copy link
Member

franky47 commented Oct 22, 2024

Ah I think I see what the problem might be.

For some TypeScript configs (that didn't support/honor export paths), the import needed to have top-level .d.ts files that point to the correct ones in the dist directory.

Looks like the ESLint rule chokes on those files, where: node_modules/nuqs/adapters/next/app.d.ts exports everything from node_modules/nuqs/dist/adapters/next/app.d.ts.

I'll see in what cases we need those intermediate top-level declaration files. If we could do without, I'd be happy, they are a pain to maintain.

What happens if you delete node_modules/nuqs/adapters/next/app.d.ts ?

@GBvanDam
Copy link
Author

GBvanDam commented Oct 22, 2024

after deleting node_modules/nuqs/adapters/next/app.d.ts it still throws:

Error while loading rule 'import/no-cycle': Package subpath './adapters/next/app' is not defined by "exports" on my end

@GBvanDam
Copy link
Author

The following eslint rules are throwing this error:

{
    'import/named': 0,
    'import/order': 0,
    'import/no-cycle': 0,
    'import/extensions': 0,
    'import/no-unresolved': 0,
    'import/no-duplicates': 0,
    'import/no-self-import': 0,
    'import/no-relative-packages': 0,
    'import/no-extraneous-dependencies': 0,
}

All of these point to the same file:
eslint-module-utils\resolve.js:233:12

/**
 * Given
 * @param p - module path
 * @param context - ESLint context
 * @return - the full module filesystem path; null if package is core; undefined if not found
 * @type {import('./resolve').default}
 */
function resolve(p, context) {
  try {
    return relative(p, getPhysicalFilename(context), context.settings);
  } catch (err) {
    if (!erroredContexts.has(context)) {
      // The `err.stack` string starts with `err.name` followed by colon and `err.message`.
      // We're filtering out the default `err.name` because it adds little value to the message.
      // @ts-expect-error this might be an Error
      let errMessage = err.message;
      // @ts-expect-error this might be an Error
      if (err.name !== ERROR_NAME && err.stack) {
        // @ts-expect-error this might be an Error
        errMessage = err.stack.replace(/^Error: /, '');
      }
      context.report({
        message: `Resolve error: ${errMessage}`,
        loc: { line: 1, column: 0 },
      });
      erroredContexts.add(context);
    }
  }
}

@acocheo
Copy link

acocheo commented Oct 23, 2024

Almost same problem here I think. Same eslint ^8.57.0 but happens also with v.^2.0.1

 Error while loading rule 'import/no-cycle': Package subpath './adapters/next/app' is not defined by "exports" in /[...]/node_modules/nuqs/package.json
 Occurred while linting [...]
    at exportsNotFound (node:internal/modules/esm/resolve:304:10)
    at packageExportsResolve (node:internal/modules/esm/resolve:594:13)
    at resolveExports (node:internal/modules/cjs/loader:634:36)
    at Module._findPath (node:internal/modules/cjs/loader:724:31)
    at findModulePath ([...]/node_modules/eslint-import-resolver-alias/index.js:99:27)
    at exports.resolve ([...]/node_modules/eslint-import-resolver-alias/index.js:75:10)
    at withResolver ([...]/node_modules/eslint-module-utils/resolve.js:180:23)
    at fullResolve ([...]/node_modules/eslint-module-utils/resolve.js:201:22)

Seems a problem of how node Module._findPath works

@franky47
Copy link
Member

Could you show me your eslint config please? I'm trying to reproduce on a fresh repo.

@acocheo
Copy link

acocheo commented Oct 23, 2024

packages:

 "eslint": "^8.41.0",
 "@next/eslint-plugin-next": "^14.0.3",
 "eslint-config-airbnb": "^19.0.4",
 "eslint-config-next": "^15.0.1",
 "eslint-config-prettier": "^9.1.0",
 "eslint-import-resolver-alias": "^1.1.2",
 "eslint-plugin-import": "^2.27.5",
 "eslint-plugin-jsdoc": "^50.2.5",
 "eslint-plugin-jsx-a11y": "^6.6.1",
 "eslint-plugin-react": "^7.31.10",
 "eslint-plugin-react-hooks": "^5.0.0"

config (I think you can cut it a bit, error is in 'import/no-cycle'):

{
    "env": {
        "browser": true,
        "commonjs": true,
        "es2021": true,
        "jest": true
    },
    "extends": [
        "airbnb",
        "plugin:jsdoc/recommended",
        "airbnb/hooks",
        "next"
    ],
    "parserOptions": {
        "ecmaVersion": "latest"
    },
    "ignorePatterns": [
        "node_modules/",
        "dist/",
        "build/",
        ".snapshots/",
        "*.min.js",
        "*.config.js"
    ],
    "rules": {
        "react/require-default-props": 0
         "max-len": ["error", {
            "code": 160,
            "ignoreStrings": true,
            "ignoreUrls": true,
            "ignoreTemplateLiterals": true,
            "ignoreRegExpLiterals": true,
            "ignoreComments": true
        }],
        "indent": ["error", 4, 
        {
            "SwitchCase": 1,
            // list derived from https://github.com/benjamn/ast-types/blob/HEAD/def/jsx.js
            "ignoredNodes": ["JSXElement", "JSXElement > *", "JSXAttribute", "JSXIdentifier", "JSXNamespacedName", "JSXMemberExpression", "JSXSpreadAttribute", "JSXExpressionContainer", "JSXOpeningElement", "JSXClosingElement", "JSXFragment", "JSXOpeningFragment", "JSXClosingFragment", "JSXText", "JSXEmptyExpression", "JSXSpreadChild"]
        }],
        "prefer-destructuring": ["error", {
            "object": true, "array": false
        }],
        "guard-for-in": "off",
        "no-plusplus" : "off",
        "no-param-reassign": "off",
        "no-console": ["error", {
            "allow": ["warn", "error"]
        }],
        "no-unused-vars": [ "error", {
            "varsIgnorePattern": "^_",
            "argsIgnorePattern": "^_",
            "args": "after-used"
        }],
        "no-underscore-dangle": ["error", {
            "allowAfterThis": true
        }],
        "no-multi-spaces": ["error", {
            "ignoreEOLComments": true
        }],
        "no-bitwise": "off",
        "no-nested-ternary": "off",
        "no-restricted-syntax": ["error", "WithStatement"],
        "no-multiple-empty-lines": ["error", {
            "max": 2, "maxBOF": 0, "maxEOF": 0
        }],
        "jsdoc/require-jsdoc": 0,
        "react/jsx-indent": ["error", 4],
        "react/jsx-indent-props": ["error", 4],
        "react/jsx-props-no-spreading": "off",
        "react/jsx-key": "error",
        "react/forbid-prop-types": "off",
        "jsx-a11y/control-has-associated-label": "off",
        "jsx-a11y/alt-text": "off",
        "jsx-a11y/click-events-have-key-events": "off",
        "jsx-a11y/no-static-element-interactions": "off",
        "jsx-a11y/interactive-supports-focus": "off",
        "jsx-a11y/anchor-has-content": "off",
        "jsx-a11y/no-noninteractive-element-interactions": "off"
    },
    "settings": {
        "import/resolver": {
            "node": true,
            "alias": {
                "extensions": [".ts", ".js", ".jsx", ".json"],
                "map": [
                    ["@/*", "./app/"],
                    ["@tests/*", "./tests/"]
                ]
            }
        }
    },
    "globals": {}
}

@franky47
Copy link
Member

With that config & packages, running eslint passes fine on a fresh install, maybe there's something else?

https://github.com/franky47/nuqs-repro-691

@GBvanDam
Copy link
Author

GBvanDam commented Oct 23, 2024

@franky47 that is strange, I cloned the exact same repo:
https://github.com/franky47/nuqs-repro-691

and did

  1. pnpm i
  2. pnpm lint

And it threw

Error while loading rule 'import/no-cycle': Package subpath './adapters/next/app' is not defined by "exports"

Further config maybe? I'm running on

  • pnpm: v9.12.2
  • npm: v10.9.0
  • node: v22.10.0
  • eslint: v8.57.1 (based on the package.json)

@franky47
Copy link
Member

Ah I see, pnpm lint is running next lint, I do get the failure with this. I was running eslint at the root of the repo (which passes).

@franky47
Copy link
Member

franky47 commented Oct 23, 2024

OK I found the issue: eslint 8 only supports CJS exports in package.json, and nuqs 2 is ESM only.

Adding this in nuqs's package.json moves the needle a bit further (even though it's plain wrong):

{
  "exports": {
    "./adapters/next/app": {
      "types": "./dist/adapters/next/app.d.ts",
      "import": "./dist/adapters/next/app.js",
      "require": "./dist/adapters/next/app.js" // Line added
    }
  }
}

@GBvanDam
Copy link
Author

GBvanDam commented Oct 23, 2024

Ahh, got it, makes sense! Are you planning on releasing a patch with this in the package.json, or do you expect us to edit the package.json locally? I saw 2.0.2 landed, which is still without

@acocheo
Copy link

acocheo commented Oct 23, 2024

Yes, the fix is correct! (damn, I saw that file but I didn't think about that..)

@franky47
Copy link
Member

I can't add this as-is to the package.json, as there isn't a valid CJS bundle to point to, and pointing to the ESM one just for ESLint to be happy seems like opening a can of worms.

I just tried replacing them with "require": null, which seems more correct, but it also fails.

Looks like updating to ESLint 9 won't help, as the error comes from eslint-import-resolver-alias which is a CJS package (and hasn't been updated in 6 years, so unlikely it moves to ESM).

Unfortunately, Node.js 23's CJS/ESM interop doesn't seem to help here either.

So yeah the best course of action would be to do a local patch-package to add this hack (we can add that to the caveats section in the docs, and link back to this issue)

@franky47
Copy link
Member

One other thing that works: pointing the "require" exports to an empty CJS file:

{
  "exports": {
    "./adapters/next/app": {
      "types": "./dist/adapters/next/app.d.ts",
      "import": "./dist/adapters/next/app.js",
      "require": "./dist/empty-file-for-eslint.cjs"
    }
  }
}

@acocheo
Copy link

acocheo commented Oct 23, 2024

Uhm, strange. Setting this line

"require": "./dist/adapters/next/app.js" // Line added

makes eslint to works correctly in my case.
And I use eslint-import-resolver-alias

@franky47
Copy link
Member

Can y'all try this?

pnpm add https://pkg.pr.new/nuqs@701

@acocheo
Copy link

acocheo commented Oct 23, 2024

It works

@GBvanDam
Copy link
Author

GBvanDam commented Oct 23, 2024

Yes! Great!

  • pnpm i
  • pnpm lint
  • pnpm build
  • pnpm start

As far as I can see no problems!

Copy link

🎉 This issue has been resolved in version 2.0.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants