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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions dist/rollup-plugin-typescript2.cjs.js
Original file line number Diff line number Diff line change
Expand Up @@ -27985,8 +27985,8 @@ const typescript = (options) => {
verbosity: VerbosityLevel.Warning,
clean: false,
cacheRoot: findCacheDir({ name: "rollup-plugin-typescript2" }),
include: ["*.ts+(|x)", "**/*.ts+(|x)"],
exclude: ["*.d.ts", "**/*.d.ts"],
include: ["*.ts+(|x)", "**/*.ts+(|x)", "**/*.cts", "**/*.mts"],
exclude: ["*.d.ts", "**/*.d.ts", "**/*.d.cts", "**/*.d.mts"],
abortOnError: true,
rollupCommonJSResolveHack: false,
tsconfig: undefined,
Expand Down Expand Up @@ -28114,7 +28114,7 @@ const typescript = (options) => {
// Rollup can't see these otherwise, because they are "emit-less" and produce no JS
if (result.references && supportsThisLoad) {
for (const ref of result.references) {
if (ref.endsWith(".d.ts"))
if (!filter(ref))
continue;
const module = yield this.resolve(ref, id);
if (!module || transformedFiles.has(module.id)) // check for circular references (per https://rollupjs.org/guide/en/#thisload)
Expand Down
2 changes: 1 addition & 1 deletion dist/rollup-plugin-typescript2.cjs.js.map

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions dist/rollup-plugin-typescript2.es.js
Original file line number Diff line number Diff line change
Expand Up @@ -27956,8 +27956,8 @@ const typescript = (options) => {
verbosity: VerbosityLevel.Warning,
clean: false,
cacheRoot: findCacheDir({ name: "rollup-plugin-typescript2" }),
include: ["*.ts+(|x)", "**/*.ts+(|x)"],
exclude: ["*.d.ts", "**/*.d.ts"],
include: ["*.ts+(|x)", "**/*.ts+(|x)", "**/*.cts", "**/*.mts"],
exclude: ["*.d.ts", "**/*.d.ts", "**/*.d.cts", "**/*.d.mts"],
abortOnError: true,
rollupCommonJSResolveHack: false,
tsconfig: undefined,
Expand Down Expand Up @@ -28085,7 +28085,7 @@ const typescript = (options) => {
// Rollup can't see these otherwise, because they are "emit-less" and produce no JS
if (result.references && supportsThisLoad) {
for (const ref of result.references) {
if (ref.endsWith(".d.ts"))
if (!filter(ref))
continue;
const module = yield this.resolve(ref, id);
if (!module || transformedFiles.has(module.id)) // check for circular references (per https://rollupjs.org/guide/en/#thisload)
Expand Down
2 changes: 1 addition & 1 deletion dist/rollup-plugin-typescript2.es.js.map

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

exclude: ["*.d.ts", "**/*.d.ts"],
include: ["*.ts+(|x)", "**/*.ts+(|x)", "**/*.cts", "**/*.mts"],
exclude: ["*.d.ts", "**/*.d.ts", "**/*.d.cts", "**/*.d.mts"],
abortOnError: true,
rollupCommonJSResolveHack: false,
tsconfig: undefined,
Expand Down Expand Up @@ -274,7 +274,7 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
// Rollup can't see these otherwise, because they are "emit-less" and produce no JS
if (result.references && supportsThisLoad) {
for (const ref of result.references) {
if (ref.endsWith(".d.ts"))
agilgur5 marked this conversation as resolved.
Show resolved Hide resolved
if (!filter(ref))
continue;

const module = await this.resolve(ref, id);
Expand Down