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

Moving VitePress' Node API as ESM only #2703

Closed
brc-dd opened this issue Jul 29, 2023 · 3 comments · Fixed by #2737
Closed

Moving VitePress' Node API as ESM only #2703

brc-dd opened this issue Jul 29, 2023 · 3 comments · Fixed by #2737
Assignees
Labels
build Related to the build system keep-open

Comments

@brc-dd
Copy link
Member

brc-dd commented Jul 29, 2023

Vite might be removing/deprecating it's CJS API is near future (vitejs/vite#13928) which will impact VitePress' CJS API as well.

Edit: Let's wait on discussion at Vite's side to conclude and we will try to follow their footsteps. But, it is recommended to use ESM APIs for anything new you might be building.

Most of the projects already appear to be using the import syntax. A quick GitHub code search shows about 20 public projects doing require('vitepress'). Exploring more, over half of them appear to be on vitepress@0.x branch. Leaving less than 10 projects that might need minor refactoring.

import syntax in config.js file when "type": "module" is not specified in nearest package.json also uses the CJS build. Might be difficult to estimate the impact. But for most of the cases where people are using import without type module or mjs/mts extension the migration should be straight forward -- either change the file extension or project level type.

Migration:

// use .mjs/.mts file extension if can't use `"type": "module"`

// change
const { defineConfig } = require('vitepress')

module.exports = {/*...*/}

// to
import { defineConfig } from 'vitepress'

export default {/*...*/}

// or since it's just a type helper simply do something like this

/** @type {ReturnType<typeof import('vitepress').defineConfig>} */
module.exports = {/*...*/}

// change
const { createMarkdownRenderer } = require('vitepress')
      // ^ or any async API like createServer, build, resolveConfig, resolveSiteData

async fn() {
  const md = await createMarkdownRenderer(/*...*/)
}

// to
async fn() {
  const { createMarkdownRenderer } = await import('vitepress')
  const md = await createMarkdownRenderer(/*...*/)
}

For maintainers, we will need to use jiti for the CLI and remove CJS stuff from rollup config.

CLI won't need any changes, it's already CJS compatible and uses ESM output. Only rollup related changes need to be done.

Rollout plan:

Let's log a warning for now if someone require's the package. We can drop the CJS API by RC.

/cc @kiaking @zonemeen

@brc-dd brc-dd added the build Related to the build system label Jul 29, 2023
@brc-dd brc-dd self-assigned this Jul 29, 2023
@brc-dd brc-dd pinned this issue Jul 29, 2023
@zonemeen
Copy link
Collaborator

I think this kind of plan works.

@brc-dd brc-dd unpinned this issue Aug 2, 2023
@benmccann
Copy link
Contributor

Most of the projects already appear to be using the import syntax. A quick GitHub code search shows about 20 public projects doing require('vitepress'). Exploring more, over half of them appear to be on vitepress@0.x branch. Leaving less than 10 projects that might need minor refactoring.

I sent PRs to a half dozen of these and filed issues with a handful more.

Let's log a warning for now if someone require's the package. We can drop the CJS API by RC.

This might end up causing multiple warnings if Vite decides to go the route of logging warnings. Honestly, it's probably easier to just go ahead and drop the CJS API now, so that you don't have to do a followup PR. I've either fixed or notified the affected projects and the error message will be a pretty common one since lots of projects are doing this migration, so there will be plenty of info out there on how to fix it.

@benmccann
Copy link
Contributor

An update from vitejs/vite#13928 - we have PRs out now to migrate all projects to ESM except for Vitepress and Storybook

@brc-dd brc-dd pinned this issue Aug 8, 2023
DrAugus referenced this issue in DrAugus/draugus.github.io Aug 11, 2023
maxnowack added a commit to maxnowack/signaldb that referenced this issue Aug 13, 2023

Verified

This commit was signed with the committer’s verified signature.
maxnowack Max Nowack
ryan4yin added a commit to ryan4yin/nixos-and-flakes-book that referenced this issue Aug 13, 2023
KzuDemEvin pushed a commit to Sybit-Education/Diveni that referenced this issue Aug 15, 2023
KzuDemEvin pushed a commit to Sybit-Education/Diveni that referenced this issue Aug 15, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
* chore(deps-dev): bump vitepress from 1.0.0-beta.7 to 1.0.0-rc.4

Bumps [vitepress](https://github.com/vuejs/vitepress) from 1.0.0-beta.7 to 1.0.0-rc.4.
- [Release notes](https://github.com/vuejs/vitepress/releases)
- [Changelog](https://github.com/vuejs/vitepress/blob/main/CHANGELOG.md)
- [Commits](vuejs/vitepress@v1.0.0-beta.7...v1.0.0-rc.4)

---
updated-dependencies:
- dependency-name: vitepress
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* fix: change node version to 18

* fix: vuejs/vitepress#2703

* fix: update name

* fix: update color scheme

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Schoch, Kevin <kevin.schoch@sybit.de>
@brc-dd brc-dd unpinned this issue Sep 17, 2023
jahow added a commit to geonetwork/geonetwork-ui that referenced this issue Sep 2, 2024
Updating vitepress required specifying the package type to module, see
vuejs/vitepress#2703
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Related to the build system keep-open
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants