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

cts/mts support #451

Merged
merged 4 commits into from Jun 23, 2023
Merged

cts/mts support #451

merged 4 commits into from Jun 23, 2023

Conversation

ezolenko
Copy link
Owner

Summary

Fix for #446

Details

Adding cts/mts to the known filetypes

@ezolenko ezolenko marked this pull request as ready for review June 23, 2023 04:08
@ezolenko ezolenko merged commit 2cb2660 into master Jun 23, 2023
12 checks passed
@agilgur5 agilgur5 added kind: feature New feature or request topic: TS version Related to a change in a TS version labels Jul 4, 2023
src/index.ts Show resolved Hide resolved
@@ -95,8 +95,8 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
verbosity: VerbosityLevel.Warning,
clean: false,
cacheRoot: findCacheDir({ name: "rollup-plugin-typescript2" }),
include: ["*.ts+(|x)", "**/*.ts+(|x)"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need to modify the TS emit output conversion to also handle .d.cts, .d.mts and their respective mappings. I assume that TS would take a CTS file and output .cjs, d.cts, and d.cts.map files, which we would not handle correctly right now.

We should also definitely have integration tests for CTS / MTS, especially to validate that assumption above^

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yeah it would fix #448 too

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh I didn't even think about that, but it would indeed fix #448. There is a slight problem there with declarations though; Rollup cannot convert them between .d.cts and .d.mts, so they are limited to the user's original module format. I'll detail that more in the issue itself.

Copy link
Collaborator

Choose a reason for hiding this comment

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

For the emit output conversion, I looked up the source code and indeed OutputFile now has some new extensions:

I imagine that with module set to output ESM (for Rollup), we'd only see the .mjs variants, but testing can confirm that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: feature New feature or request topic: TS version Related to a change in a TS version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants