-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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(core): support TypeScript + ESM configuration #9317
Conversation
Thanks for working on this. Didn't know Jiti but looks nice 👍 Note we are also happy to drop usage of import-fresh for a few reasons:
|
Co-authored-by: Joshua Chen <[email protected]>
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site configuration. |
⚡️ Lighthouse report for the deploy preview of this PR
|
I assume fix PR fix #5379 right? We should probably move our init templates to use ESM and/or TypeScript configs, and keep a e2e test using CJS for retro-compatibility I'd be happy to have this in v3! |
Yes, ESM should be supported as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately it doesn't seem to take into account updates
I tried to use a sidebars.ts
file, it works the first time, but any live edit to this file are not reloaded unless you fully restart the dev server.
Any idea if your package can serve as a replacement for importFresh
@pi0 ? I thought cache: false
would do the job but apparently not.
return importFresh(sidebarFilePath); | ||
const sidebars = jiti(__filename, { | ||
cache: false, | ||
interopDefault: true, | ||
debug: true, | ||
})(sidebarFilePath); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not look like a drop-in replacement for importFresh
unfortunately.
Calling it multiple times will return the same result despite changing the underlying file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi. You can set requireCache: false
in order to disable (runtime) cache. cache
option is for filesystem transform cache (and can be even kept enabled for better performance. you still will get a fresh value each time).
Please let me know if could help further on this :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, we are working on new API jiti.import()
(releasing by next week) if PR was delayed until then would be nice that you consider the migration for better future compatibility. It can bring support more native ESM compatibilities.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot @pi0 that's working. I saw requireCache
in an older issue but thought it was an older removed option because it's not in v1 or v2 docs. But it's in the type and seems to work as I expect 👍
Regarding jiti.import()
(unjs/jiti#174?) I assume you want to say v2.0 is being released soon?
On my side I didn't expect to migrate away from importFresh
in Docusaurus v3 but as jiti seems to work that would be a awesome improvement for our users so I'm already delaying a bit our official release to incorporate this PR 😄 I'd prefer to not be an early adopter of jiti v2 this time, for timing reasons it's preferable if we upgrade for Docusaurus v4 (or in a minor version later, if not considered as a breaking change)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah awesome!
jiti.import()
API will be also released as a backward compatible stub for v1 for future compatibility so you can migrate earlier and only bump the major version of jiti later -- which i expect no major breaking changes. bumping to v2 is to make sure ecosystem migration is not risky. (i am on holidays with family RN but might find some time to do it earlier). v2 is not expected that soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks :)
From what I understand currently Jiti is able to load modules synchronously but import will be async so it's probably better to assume async usage in the future?
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
LGTM 🥳 thanks for the POC @harryzcy , everything works fine now and super happy to have this PR ready just in time for Docusaurus v3 👍 |
Breaking change
Change dependency that loads config files such as:
docusaurus.config.js
sidebars.js
plugin.js
preset.js
It is now possible to write those files in ESM or TypeScript, and they will be lazily transpiled if needed.
As any major deps change, some subtle breaking changes could happen, although we expect your former CJS config files to keep working there's no strict guarantee and some adaptations might be required.
Limitations
Top-level await is not yet supported (issue unjs/jiti#72)
Motivation
Allow configuration files to be defined in modern ESM or TS
Test Plan
CI
Test links
Deploy preview: https://deploy-preview-9317--docusaurus-2.netlify.app/
Related issues/PRs
fix #5379
fix #7911