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(build): switch to vite 3 and support clean urls #856

Merged
merged 125 commits into from Aug 17, 2022
Merged

Conversation

brc-dd
Copy link
Member

@brc-dd brc-dd commented Jun 25, 2022

@brc-dd brc-dd marked this pull request as ready for review June 25, 2022 19:28
@brc-dd brc-dd changed the title chore: switch to vite 3 feat(build): switch to vite 3 Jun 25, 2022
src/node/alias.ts Show resolved Hide resolved
src/node/alias.ts Show resolved Hide resolved
@yyx990803
Copy link
Member

I've tested this branch in the main Vue docs and aside the from /@theme alias removal, everything else seems to work fine.

LGTM after the Vue alias resolution is restored.

@brc-dd brc-dd marked this pull request as draft August 16, 2022 04:52
brc-dd and others added 6 commits August 16, 2022 17:35
Background: pnpm injects `NODE_PATH` when installing npm script binaries
in order to simulate flat install structure when running npm scripts.
This previously made files outside of VitePress to be able to import
transitive deps (e.g. `vue`), but this breaks when upgrading to Vite 3
or in esm mode, because:

- "type": "module", aka ESM mode doesn't support `NODE_PATH`, so now
  project files can't resolve `vue` which is a transitive dep.

- Vite 3 now auto-resolves SSR externals, but it requires the
  dep to be resolvable first. Since it can't resovle `vue`, the Rollup
  build will fail.

The fix: detect if `vue` is resolvable from project root's node_modules.
If not, create a symlink to the version of `vue` from VitePress' own
deps.
@yyx990803
Copy link
Member

yyx990803 commented Aug 17, 2022

Since we are going to squash the commits, leaving a note about the fix in 3b2d90a:

Summary

  1. Do not alias Vue during SSR build (since all deps are expected to be externalized with Vite 3)
  2. Before SSR build, link vue to project root node_modules if doesn't exist so that it can be resolved from project files
  3. Move renderToString import from vue/server-renderer into the bundled SSR app to avoid duplicated copies of Vue during SSR

Background: pnpm injects NODE_PATH when installing npm script binaries
in order to simulate flat install structure when running npm scripts.
This previously made files outside of VitePress, e.g. .md files in project root to
be able to import VitePress' transitive deps (e.g. vue). However this breaks
when upgrading to Vite 3 or when building in esm mode, because:

  • "type": "module", aka ESM mode doesn't support NODE_PATH, so now
    project files can't resolve vue which is a transitive dep.

  • Vite 3 now auto-resolves SSR externals, but it requires the
    dep to be resolvable first. Since it can't resovle vue, the Rollup
    build will fail.

The fix: detect if vue is resolvable from project root's node_modules.
If not, create a symlink to the version of vue from VitePress' own
deps.

This way we achieve the desired result:

  • The default theme can work without user explicitly installing vue as a peer dep
  • If the user installs vue in project root, always use the user's installed version in all cases.

Another point worth mentioning: the reason we have fails like "app data not properly injected" means there are two different copies of Vue being used during SSR. The most likely reason for this is the version of vue/server-renderer loaded in src/node/build/render.ts by VitePress isn't the same copy imported by the loaded SSR app.js. To avoid the problem I moved the logic of calling renderToString into the bundled SSR app itself, so VitePress simply calls a function exported by the app.

@yyx990803 yyx990803 marked this pull request as ready for review August 17, 2022 08:31
@yyx990803 yyx990803 merged commit 0048e2b into main Aug 17, 2022
@Lexpeartha
Copy link

Not sure if it's related, but after updating to alpha 6, sidebar just disappeared, even tho I have aside set to true?

@brc-dd
Copy link
Member Author

brc-dd commented Aug 17, 2022

@Lexpeartha can you create an issue and tell us how to reproduce that? Also, aside is for the right-side outline, ads thing and it's true by default.

@brc-dd brc-dd deleted the chore/vite-3 branch August 17, 2022 08:51
@Lexpeartha
Copy link

You can checkout my repo which had that right-side outline before bumping to the new alpha. I'm assuming something changed internally, since I changed nothing in the actual markdown?

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.