Skip to content

Conversation

YangJonghun
Copy link
Contributor

Description

If developer forces module type via specifing TypeScript extension,
Make JS extension follow TS extension,
Node.js read file's module correctly even if it doesn't match "type" field within package.json

This PR can resolve below Node.js warning (when use rolldown.config.mts)

(node:885) [MODULE_TYPELESS_PACKAGE_JSON] Warning: Module type of file:///<path_name>/rolldown.config.[hash].js is not specified and it doesn't parse as CommonJS.
Reparsing as ES module because module syntax was detected. This incurs a performance overhead.
To eliminate this warning, add "type": "module" to <path_name>/package.json.
(Use `node --trace-warnings ...` to show where the warning was created)

@underfin
Copy link
Contributor

underfin commented Mar 4, 2025

Could you give a reproduction for it, I'm not sure what happened at here.

@IWANABETHATGUY
Copy link
Member

Can not reproduce it either,
image

@Joery-M
Copy link

Joery-M commented Mar 5, 2025

I believe I managed to reproduce:
Remove "type": "module" from package.json, and change the config file extension from .[cm]ts to just .ts (or .js).
https://stackblitz.com/edit/github-ja4hyvvr?file=package.json

In stackblitz this errors out since it uses node 18 (tries to parse as cjs), but the exact warning can be reproduced when using node v22.14 (not specifically, just what I had on hand)

$ nr build

> build
> rolldown -c ./rolldown.config.js

(node:22952) [MODULE_TYPELESS_PACKAGE_JSON] Warning: Module type of file:///<folder>/rolldown.config.js is not specified and it doesn't parse as CommonJS.
Reparsing as ES module because module syntax was detected. This incurs a performance overhead.
To eliminate this warning, add "type": "module" to <folder>\package.json.
(Use `node --trace-warnings ...` to show where the warning was created)
<DIR>/other-entry.js    chunk │ size: 0.11 kB
<DIR>/cube-DoVzd1lL.js  chunk │ size: 0.17 kB
<DIR>/entry.js          chunk │ size: 0.20 kB

✔ Finished in 18.54 ms

Do note that I don't believe this PR fixes the warning, but I could be mistaken in that regard.

@YangJonghun
Copy link
Contributor Author

YangJonghun commented Mar 5, 2025

Reproduction is the same as @Joery-M mentioned.
Remove "type" field from package.json, and set config file to .mts extension, also it reproduces on node v22.14.0

This PR aim for no "type": "module" declaration and only removes Node.js warning when using .mts file as configuration.

Regardless of the value of the "type" field, .mjs files are always treated as ES modules and .cjs files are always treated as CommonJS.

If .ts extension is used without filling out "type" field, developer has not explicitly specified module format, so it is correct behavior for Node.js to output warning

@underfin underfin self-requested a review March 6, 2025 04:23
@underfin underfin self-assigned this Mar 6, 2025
@underfin underfin removed their request for review March 6, 2025 04:29
@underfin
Copy link
Contributor

underfin commented Mar 6, 2025

Thanks for your description. I updated the pr to add the tests, and also fixed load the cts error, it need to compiled the cts to cjs.

@underfin underfin enabled auto-merge March 6, 2025 07:10
@underfin underfin changed the title refactor(node): maintain module definition of TS extension as is fix: load mts config without type module Mar 6, 2025
@underfin underfin added this pull request to the merge queue Mar 6, 2025
Merged via the queue into rolldown:main with commit 64998fb Mar 6, 2025
18 checks passed
@YangJonghun YangJonghun deleted the refactor/config-ext-transform branch March 6, 2025 11:18
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.

4 participants