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: support importing json modules #336

Merged
merged 9 commits into from
Feb 26, 2025

Conversation

kricsleo
Copy link
Contributor

resolves #332
resolves #184

(Can also resolve some downstream issues: unjs/unbuild#430)

@kricsleo
Copy link
Contributor Author

This is so strange that the CI test has been returning inconsistent results for testcases/import-json.
Sometimes the result is declare let name: ... and sometimes it's declare const name: ....
I don't have this issue locally.

@kricsleo kricsleo changed the title Feat/support json module feat: support importing json modules Feb 19, 2025
@Swatinem
Copy link
Owner

The testsuite runs twice, once with the "latest" (aka the declared dependency version of the typescript compiler, which might need a bump in either case).
And then a second time with the "minimum supported TS version", which is 4.5.

That second run fails, whereas the first one succeeds. Some somewhere in between the TS compiler itself changed how it generates declarations for JSON.

You can add the minimum TS version the test should run with in the meta file. Feel free to just bump that to the latest version, otherwise you can also bisect the specific TS version that did this change, but thats probably not worth the effort.

@kricsleo
Copy link
Contributor Author

Thanks for the detailed explanation ❤️
I tried to bisect and determine that this behaviour was caused by microsoft/TypeScript#53206.

The impacted official version is v5.1.3.

  • < v5.1.3(The closest version: 5.0.4): declare const name: ....
  • >= v5.1.3declare let name: ....

I'm not sure if it's meant to change the behavior of JSON modules, but since it doesn't impact actual usage, I'll let it be. 👌🏻

@Swatinem Swatinem merged commit 3348b0f into Swatinem:master Feb 26, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect d.ts When using the json plugin and reexporting a json file Rollup fails if I import a json file
2 participants