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

feat(rspack_core): configurable tsconfig project references #4290

Merged
merged 1 commit into from
Oct 10, 2023

Conversation

Boshen
Copy link
Contributor

@Boshen Boshen commented Oct 8, 2023

closes #4211

Summary

See #4211 for background.

This PR aligns the tsconfig option with https://github.com/dividab/tsconfig-paths-webpack-plugin#options

The original resolve#tsConfigPath option is not removed nor deprecated in this PR.

For better ux, the tsconfig#references option has an additional auto value where the values of project references can be automatically picked up from tsconfig.json.

Test Plan

Two test cases are added to rspack tests:

  • packages/rspack/tests/configCases/resolve-new/tsconfig-project-references-auto: uses project references defined in tsconfig.json
  • packages/rspack/tests/configCases/resolve-new/tsconfig-project-references-manual: uses project references defined in webpack.config.js#resolve#references.

Require Documentation?

  • No
  • Yes, the corresponding rspack-website PR is TODO @Boshen

@Boshen Boshen added the need documentation Create a tracking issue in rspack-website label Oct 8, 2023
@Boshen Boshen requested review from a team October 8, 2023 11:30
@Boshen Boshen requested a review from h-a-n-a as a code owner October 8, 2023 11:30
@github-actions github-actions bot added release: feature team The issue/pr is created by the member of Rspack. labels Oct 8, 2023
@hardfist
Copy link
Contributor

hardfist commented Oct 8, 2023

!canary

@hardfist
Copy link
Contributor

hardfist commented Oct 8, 2023

@chenjiahan can you help test it in modernjs sourceBuild tests

hardfist
hardfist previously approved these changes Oct 8, 2023
@chenjiahan
Copy link
Member

I'd love to, can you provide a canary version?

@chenjiahan
Copy link
Member

Or you can merge it and we will test it tomorrow.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2023

0.3.6-canary-9cae447-20231008135313

@hardfist
Copy link
Contributor

hardfist commented Oct 8, 2023

0.3.6-canary-9cae447-20231008135313

here you are @chenjiahan

@chenjiahan
Copy link
Member

I'm trying to use the new resolver and references in Modern.js, but I got this error:

image

@Boshen can you help to figure it out?

Reproduce steps:

git clone [email protected]:web-infra-dev/modern.js.git

git checkout tsconfig_references_1009

pnpm i

cd tests/integration/source-code-build/app

PROVIDE_TYPE=rspack pnpm dev

@Boshen
Copy link
Contributor Author

Boshen commented Oct 9, 2023

@chenjiahan There is a small bug related to resolving extends, try "extends": "@modern-js/tsconfig/base.json" for the meantime while I patch a fix.

After this is fixed, it shows that react/jsx-dev-runtime cannot be resolved:

image

This is due to react being installed in source-code-build/app/node_modules, so it won't be found in source-code-build/components.

@chenjiahan
Copy link
Member

chenjiahan commented Oct 9, 2023

This is due to react being installed in source-code-build/app/node_modules, so it won't be found in source-code-build/components.

Got it, but when Modern.js using webpack, the react/jsx-dev-runtime can be resolved correctly, i have no idea why it can work...

image

@Boshen
Copy link
Contributor Author

Boshen commented Oct 9, 2023

@chenjiahan Is it because you specified "@source-code-build/components": "workspace:*" in your package.json#dependencies so it read the import @source-code-build/components from source-code-build/app/node_modules/@source-code-build/components/src/card/index.tsx instead of source-code-build/components/src/card/index.tsx?

I'm not really up to date around workspace dependencies and project references ...

@chenjiahan
Copy link
Member

When using workspace:*, the package is linked by symlink, so it should be resolved to the real path by default (according to https://webpack.js.org/configuration/resolve/#resolvesymlinks)

@Boshen
Copy link
Contributor Author

Boshen commented Oct 9, 2023

@chenjiahan I ran pnpm dev and it threw

File: ./src/App.tsx
Module not found: Can't resolve '@source-code-build/components' in '/Users/bytedance/github/modern.js/tests/integration/source-code-build/app/src'

How did you make it run successfully with webpack?

@chenjiahan
Copy link
Member

Yes it can work in webpack. You need to enable the following config:

image

Or just checkout to the master branch of modern.js.

@Boshen
Copy link
Contributor Author

Boshen commented Oct 9, 2023

@chenjiahan webpack works because the app's node_modules directory is manually added to resolve#modules.

If you print out normalizedOptions.modules from enhanced-resolve located in node_modules/.pnpm/[email protected]/node_modules/enhanced-resolve/lib/ResolverFactory.js, you get

[
  'node_modules',
  '/Users/bytedance/github/modern.js/tests/integration/source-code-build/app/node_modules'
]

This resolver#modules value is not passed to Rspack's resolve option so it didn't build successfully. I verified this manually by adding it to rspack#resolve and it builds successfully.

@chenjiahan
Copy link
Member

Nice catch! This is a difference between the webpack and rspack modes of modern.js, I will fix it.

@Boshen
Copy link
Contributor Author

Boshen commented Oct 10, 2023

Are there any more blockers?

@chenjiahan
Copy link
Member

Time to merge! I will do some more testing later.

@hardfist hardfist enabled auto-merge October 10, 2023 06:24
@hardfist hardfist added this pull request to the merge queue Oct 10, 2023
Merged via the queue into main with commit c0965f1 Oct 10, 2023
15 checks passed
@hardfist hardfist deleted the resolver-tsconfig-references branch October 10, 2023 07:09
@hardfist hardfist added the release: feature release: feature related release(mr only) label Oct 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need documentation Create a tracking issue in rspack-website release: feature release: feature related release(mr only) team The issue/pr is created by the member of Rspack.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

need an option to control whether enable or disable project reference
3 participants