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: support .cts and .mts #3831

Merged
merged 4 commits into from
Jun 3, 2023
Merged

fix: support .cts and .mts #3831

merged 4 commits into from
Jun 3, 2023

Conversation

alexander-akait
Copy link
Member

@alexander-akait alexander-akait commented Jun 3, 2023

Some magic:

  • handle .cts and .mts
  • .mts is still have hacky support - NODE_OPTIONS="--loader ts-node/esm" webpack-cli --config ./webpack.config.mts
  • .cts works fine
  • .ts works out of box for CommonJS, but need a hack from above for .mts
  • improve perf for .mts to avoid using extra require

@hardfist there is basic support for .ctsand .mts (.ts already supported), no problems with commonjs, but esm using is still hacky due rechoir doesn't support ESM hooks for Node.js (because Node.js changed it multiple times recently and it is still not stable).

What kind of change does this PR introduce?

I want to say it is a bugfix

Did you add tests for your changes?

Soon

If relevant, did you update the documentation?

No need

Summary

Better support typescript configurations

Does this PR introduce a breaking change?

No

Other information

No

@codecov
Copy link

codecov bot commented Jun 3, 2023

Codecov Report

Merging #3831 (bfddce3) into master (73bd2cc) will decrease coverage by 0.23%.
The diff coverage is 85.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3831      +/-   ##
==========================================
- Coverage   91.64%   91.42%   -0.23%     
==========================================
  Files          22       22              
  Lines        1628     1667      +39     
  Branches      466      480      +14     
==========================================
+ Hits         1492     1524      +32     
- Misses        136      143       +7     
Impacted Files Coverage Δ
packages/webpack-cli/src/webpack-cli.ts 92.66% <85.00%> (-0.40%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 73bd2cc...bfddce3. Read the comment docs.

@alexander-akait
Copy link
Member Author

And I really hope we will not have .mjsx, .cjsx (and same for typescript), it will be a disaster 🤯

}
break;
}
case "esm": {
Copy link
Member Author

Choose a reason for hiding this comment

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

Avoid extra require for esm (i.e. mts and mjs)

@@ -1,7 +1,7 @@
path = require 'path'

config =
mode: 'production'
mode: 'development'
Copy link
Member Author

Choose a reason for hiding this comment

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

For faster tests

".webpack/webpack.config",
".webpack/webpackfile",
].flatMap((filename) => extensions.map((ext) => path.resolve(filename + ext)));
const defaultConfigFiles = new Set(
Copy link
Member Author

Choose a reason for hiding this comment

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

To avoid duplicates

@alexander-akait alexander-akait enabled auto-merge (squash) June 3, 2023 23:11
@alexander-akait alexander-akait merged commit a77daf2 into master Jun 3, 2023
53 of 55 checks passed
@alexander-akait alexander-akait deleted the feat-mts-support branch June 3, 2023 23:45
@hardfist
Copy link

hardfist commented Jun 6, 2023

the current method seems good enough for normal use case and we're going to support this as well

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

Successfully merging this pull request may close these issues.

None yet

3 participants