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

fix(core): Respect extends in local plugin TS options #30062

Merged
merged 4 commits into from
Feb 18, 2025

Conversation

ethantkoenig
Copy link
Contributor

Fix a bug introduced in #29774.

Current Behavior

If a local plugin's tsconfig.json uses extends, the referenced base config is not loaded.

Expected Behavior

If a local plugin's tsconfig.json uses extends, the referenced base config is loaded.

Related Issue(s)

Fixes #30007

Copy link

vercel bot commented Feb 16, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nx-dev ✅ Ready (Inspect) Visit Preview Feb 18, 2025 9:16am

Copy link

nx-cloud bot commented Feb 16, 2025

View your CI Pipeline Execution ↗ for commit b6fbb59.

Command Status Duration Result
nx affected --targets=lint,test,build,e2e,e2e-c... ✅ Succeeded 36m 12s View ↗
nx run-many -t check-imports check-commit check... ✅ Succeeded 15s View ↗
nx-cloud record -- nx-cloud conformance:check ✅ Succeeded 2s View ↗
nx-cloud record -- nx format:check --base= --he... ✅ Succeeded 5s View ↗
nx-cloud record -- nx sync:check ✅ Succeeded 4s View ↗
nx documentation ✅ Succeeded 2m 57s View ↗

☁️ Nx Cloud last updated this comment at 2025-02-18 09:50:40 UTC

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@ethantkoenig ethantkoenig marked this pull request as ready for review February 17, 2025 00:32
@ethantkoenig ethantkoenig requested a review from a team as a code owner February 17, 2025 00:32
@alumni
Copy link

alumni commented Feb 17, 2025

TypeError: s.codePointAt is not a function
    at codePointAt (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:14500:12)
    at codePointUnchecked (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:12073:12)
    at Object.scan (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:12684:18)
    at nextTokenWithoutCheck (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:33362:36)
    at nextToken (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:33372:12)
    at Object.parseJsonText2 [as parseJsonText] (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:32931:5)
    at parseJsonText (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:32759:17)
    at parseConfigFileTextToJson (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:42125:26)
    at readJsonOrUndefined (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:20956:18)
    at readJson (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:20960:10)
    at getPackageJsonInfo (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:45312)
    at <anonymous> (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:45244)
    at <anonymous> (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:45910)
    at forEachAncestorDirectory (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:9055)
    at forEachAncestorDirectoryStoppingAtGlobalCache (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:45909)
    at getPackageScopeForPath (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:45241)
    at loadModuleFromSelfNameReference (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:45412)
    at tryResolve (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:44888)
    at nodeModuleNameResolverWorker (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:44824)
    at nodeNextJsonConfigResolver (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:44772)
    at getExtendsConfigPath (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:43181)
    at getExtendsConfigPathOrArray (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:43078)
    at parseOwnConfigOfJson (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:43071)
    at parseConfig (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:43012)
    at parseJsonConfigFileContentWorker (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:42739)
    at Object.parseJsonConfigFileContent (<root>/node_modules/.pnpm/typescript@5.7.3/node_modules/typescript/lib/typescript.js:42691)
    at readTsConfigOptions (<root>/node_modules/.pnpm/nx@20.4.4_patch_hash=elh2fg32mryls3fzupnjo5fmoq_@swc-node+register@1.10.9_@swc+core@1.10.12_@_oj4vdjnizaxj3eefmlqqciq44y/node_modules/nx/src/plugins/js/utils/typescript.js:32)
    at registerPluginTSTranspiler (<root>/node_modules/.pnpm/nx@20.4.4_patch_hash=elh2fg32mryls3fzupnjo5fmoq_@swc-node+register@1.10.9_@swc+core@1.10.12_@_oj4vdjnizaxj3eefmlqqciq44y/node_modules/nx/src/project-graph/plugins/transpiler.js:27)
    at <anonymous> (<root>/node_modules/.pnpm/nx@20.4.4_patch_hash=elh2fg32mryls3fzupnjo5fmoq_@swc-node+register@1.10.9_@swc+core@1.10.12_@_oj4vdjnizaxj3eefmlqqciq44y/node_modules/nx/src/config/schema-utils.js:23)
    at runExecutorInternal (<root>/node_modules/.pnpm/nx@20.4.4_patch_hash=elh2fg32mryls3fzupnjo5fmoq_@swc-node+register@1.10.9_@swc+core@1.10.12_@_oj4vdjnizaxj3eefmlqqciq44y/node_modules/nx/src/command-line/run/run.js:97)
    at process.processTicksAndRejections (<node_internals>/internal/process/task_queues:95)
    at await (Unknown Source:0)
    at <anonymous> (<root>/node_modules/.pnpm/nx@20.4.4_patch_hash=elh2fg32mryls3fzupnjo5fmoq_@swc-node+register@1.10.9_@swc+core@1.10.12_@_oj4vdjnizaxj3eefmlqqciq44y/node_modules/nx/src/command-line/run/run.js:176)
    at handleErrors (<root>/node_modules/.pnpm/nx@20.4.4_patch_hash=elh2fg32mryls3fzupnjo5fmoq_@swc-node+register@1.10.9_@swc+core@1.10.12_@_oj4vdjnizaxj3eefmlqqciq44y/node_modules/nx/src/utils/handle-errors.js:8)
    at run (<root>/node_modules/.pnpm/nx@20.4.4_patch_hash=elh2fg32mryls3fzupnjo5fmoq_@swc-node+register@1.10.9_@swc+core@1.10.12_@_oj4vdjnizaxj3eefmlqqciq44y/node_modules/nx/src/command-line/run/run.js:174)
    at <anonymous> (<root>/node_modules/.pnpm/nx@20.4.4_patch_hash=elh2fg32mryls3fzupnjo5fmoq_@swc-node+register@1.10.9_@swc+core@1.10.12_@_oj4vdjnizaxj3eefmlqqciq44y/node_modules/nx/bin/run-executor.js:59)
    at emit (<node_internals>/events:518)
    at process.emit (<root>/node_modules/.pnpm/nx@20.4.4_patch_hash=elh2fg32mryls3fzupnjo5fmoq_@swc-node+register@1.10.9_@swc+core@1.10.12_@_oj4vdjnizaxj3eefmlqqciq44y/node_modules/nx/src/native/index.js:20)

Copy link
Member

@leosvelperez leosvelperez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for contributing!

The change makes sense. I just requested a couple of changes to the test file but nothing major. If you don't have time to address them, please let me know and I'll update the PR.

@alumni
Copy link

alumni commented Feb 17, 2025

Thank you for contributing!

The change makes sense. I just requested a couple of changes to the test file but nothing major. If you don't have time to address them, please let me know and I'll update the PR.

@leosvelperez it still fails for workspace plugins (see the stack trace above).

Edit 1: While debugging I noticed the issue is not with the tools/workspace-plugin/tsconfig.json anymore, but with tsconfig.base.json which extends from @tsconfig/node20/tsconfig.json.

Edit 2: Somehow it fails when parsing the package.json of the monorepo.

Edit 3: I noticed directoryExists was needed, so I added that to host, but it started to break somewhere else. I think resolving the TSConfig relies on the entire ParseConfigHost, it's not something we can avoid.

@leosvelperez
Copy link
Member

leosvelperez commented Feb 17, 2025

@alumni thanks for the extra feedback and testing!

@ethantkoenig we should update the host to:

const host: Partial<ts.ParseConfigHost> = {
  ...tsModule.sys,
  readDirectory: () => [],
};

That way, we have everything that's needed to read the config files (including extended config files from third-party packages) while still avoiding scanning directories, which are only used to identify the fileNames property in the parsed configuration.

Please add extra test cases that cover extending from third-party packages.

@alumni
Copy link

alumni commented Feb 17, 2025

const host: Partial<ts.ParseConfigHost> = {
  ...tsModule.sys,
  readDirectory: () => [],
};

Seems to work for the project config that I have.

Partial might not be necessary, ParseConfigHost seems to be a subset of System.

Thanks both :)

@ethantkoenig
Copy link
Contributor Author

@ethantkoenig we should update the host to:

const host: Partial<ts.ParseConfigHost> = {
  ...tsModule.sys,
  readDirectory: () => [],
};

That way, we have everything that's needed to read the config files (including extended config files from third-party packages) while still avoiding scanning directories, which are only used to identify the fileNames property in the parsed configuration.

Done

Please add extra test cases that cover extending from third-party packages.

Added a test, though I was unable to come up with a third-party test case that fails with the old code, so the test might not be super useful. @alumni if you have a minimal reproducible example you are able to easily share, that would be helpful.

@leosvelperez leosvelperez enabled auto-merge (squash) February 18, 2025 09:08
@leosvelperez leosvelperez enabled auto-merge (squash) February 18, 2025 09:21
@leosvelperez leosvelperez merged commit 9cdc1cc into nrwl:master Feb 18, 2025
12 checks passed
jaysoo pushed a commit that referenced this pull request Feb 18, 2025
Fix a bug introduced in #29774.

## Current Behavior

If a local plugin's `tsconfig.json` uses `extends`, the referenced base
config is not loaded.

## Expected Behavior

If a local plugin's `tsconfig.json` uses `extends`, the referenced base
config is loaded.

## Related Issue(s)

Fixes #30007

---------

Co-authored-by: Leosvel Pérez Espinosa <leosvel.perez.espinosa@gmail.com>
Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default imports broken in custom executor in 20.4.1
3 participants