-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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: migrate to ESM (#645) #659
Conversation
"compilerOptions": { | ||
"baseUrl": ".", | ||
"outDir": "../../dist/client", | ||
"target": "esnext", | ||
"module": "esnext", | ||
"declaration": true, | ||
"declarationDir": "../../dist/client-types", |
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 should be client
instead of client-types
?
"declarationDir": "../../dist/client-types", | |
"declarationDir": "../../dist/client", |
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.
Wait not really, we need client-types
in rollup config... However, this raises
[!] (plugin cleanup) Error: ENOENT: no such file or directory, stat '/Users/kia/Code/vitepress-next/dist/client-types'
When running pnpm run docs
.
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.
OK this is happening because on dev
command, client build executes, and then node build (rollup) executes in parallel and client-types
folder gets deleted becaue of cleanup
hook in rollup config. So we should first check for the folder existense and then perform delete on the folder I guess.
So maybe we just do this?
// rollup.config.ts
plugins: [
dts(),
{
name: 'cleanup',
async closeBundle() {
try {
await fs.rm(r('dist/client-types'), { recursive: true })
} catch {}
}
}
]
@antfu Currently, because we only have es build, the consumer of VitePress have to define Maybe we need |
I think I'll add cjs support. Tested locally with Vite and it will work with/without |
Opened PR that targets this PR at #666. |
This is awesome! Related vitejs/vite#8348 (comment). I think we should be able to test VitePress with the new ESM SSR build after this PR is merged. |
Cool 👀 I'm resolving conflict now and will merge it soon 👍 |
close #645
This shouldn't have much impact on the user side as Vitepress is mostly a CLI tool
Changes
"type": "module"
and now.js
are all ESMrollup-plugin-dts
instead of api-extractor for simplitypicocolors
,fast-glob
,simple-git-hooks
andprompts
to align with vite>=14.6.0
to match with Vite