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: add --config-loader CLI option #9210

Merged
merged 5 commits into from
Feb 11, 2025

Conversation

krutoo
Copy link
Contributor

@krutoo krutoo commented Feb 8, 2025

Summary

According to my issue #9176 I prepare PR with implementation of --disable-interpret CLI option.

This CLI option is analogue of same option in Webpack.

For myself this option is wery helpful because in my projects I use tsimp as main loader for TS.

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

Copy link

netlify bot commented Feb 8, 2025

Deploy Preview for rspack canceled.

Name Link
🔨 Latest commit 8ff1680
🔍 Latest deploy log https://app.netlify.com/sites/rspack/deploys/67ab2f2eb534e800088ff37a

@krutoo krutoo changed the title #9176 feat: --disable-interpret CLI option feat: --disable-interpret CLI option Feb 8, 2025
@github-actions github-actions bot added the release: feature release: feature related release(mr only) label Feb 8, 2025
Copy link
Member

@chenjiahan chenjiahan 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 your contribution!

Since interpret is no longer maintained, Rspack will most likely replace interpret with other solutions in the future. Therefore we prefer a more general flag name.

How about --config-loader native? Both Rsbuild and Vite provides --config-loader native flag.

See:

Copy link

codspeed-hq bot commented Feb 8, 2025

CodSpeed Performance Report

Merging #9210 will not alter performance

Comparing krutoo:cli-disable-interpret (8ff1680) with main (c1e945f)

Summary

✅ 6 untouched benchmarks

@krutoo
Copy link
Contributor Author

krutoo commented Feb 8, 2025

@chenjiahan sounds good, do I need to wait for someone else's opinion (from maintainers of this repo) or can I remake the option to --loader-config?

@chenjiahan
Copy link
Member

You can remake the options first.
I'll invite other maintainers if needed.

@krutoo krutoo changed the title feat: --disable-interpret CLI option feat: --config-loader CLI option Feb 10, 2025
@krutoo
Copy link
Contributor Author

krutoo commented Feb 10, 2025

@chenjiahan would it be more correct to use --config-loader interpret instead of --config-loader native?

As I understand "native" means using --strip-types (native Node.js feature to partially use TS)

@chenjiahan chenjiahan changed the title feat: --config-loader CLI option feat: add --config-loader CLI option Feb 11, 2025
chenjiahan
chenjiahan previously approved these changes Feb 11, 2025
@chenjiahan chenjiahan enabled auto-merge (squash) February 11, 2025 09:26
@chenjiahan chenjiahan merged commit 3fa4f3e into web-infra-dev:main Feb 11, 2025
30 checks passed
@krutoo
Copy link
Contributor Author

krutoo commented Feb 11, 2025

@chenjiahan should I also update docs?

@chenjiahan
Copy link
Member

That would be helpful 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release: feature release: feature related release(mr only)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants