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

fix(deps): update dependency vite to v5 #10167

Closed
wants to merge 23 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 8 additions & 1 deletion .github/workflows/ci.yml
Expand Up @@ -569,7 +569,10 @@ jobs:
REDWOOD_TEST_PROJECT_PATH: ${{ steps.set-up-rsc-project.outputs.rsc-project-path }}
REDWOOD_DISABLE_TELEMETRY: 1

# TODO: This workflow times out on Windows. It looks as if the dev server starts up ok,
# but we can't tell what the browser is rendering.
- name: 🐘 Run RSC dev smoke tests
if: matrix.os == 'ubuntu-latest'
working-directory: tasks/smoke-tests/rsc-dev
run: npx playwright test
env:
Expand Down Expand Up @@ -683,7 +686,7 @@ jobs:

- name: Build for production
working-directory: ${{ steps.set-up-test-project.outputs.test-project-path }}
run: yarn rw build --no-prerender
run: yarn rw build --no-prerender --verbose
env:
REDWOOD_DISABLE_TELEMETRY: 1

Expand All @@ -694,6 +697,10 @@ jobs:
REDWOOD_TEST_PROJECT_PATH: '${{ steps.set-up-test-project.outputs.test-project-path }}'
REDWOOD_DISABLE_TELEMETRY: 1

- name: Setup tmate session
if: ${{ failure() }}
uses: mxschmitt/action-tmate@v3

ssr-smoke-tests-skip:
needs: detect-changes
if: needs.detect-changes.outputs.ssr == 'false'
Expand Down
4 changes: 4 additions & 0 deletions packages/cli/src/commands/buildHandler.js
Expand Up @@ -134,6 +134,10 @@ export const handler = async ({
// it could affect other things that run in parallel while building.
// We don't have any parallel tasks right now, but someone might add
// one in the future as a performance optimization.
//
// Disable the new warning in Vite v5 about the CJS build being deprecated
// so that users don't have to see it when this command is called with --verbose
process.env.VITE_CJS_IGNORE_WARNING = 'true'
Comment on lines +137 to +140
Copy link
Contributor

@jtoar jtoar Mar 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vite 5 deprecated the CJS api. The only place the CJS api is used is in a user's vite.config.ts file. We can disable the warning with https://github.com/vitejs/vite/blob/504bc5f80888ebd89d6c6726b18cb6f322ed538f/packages/vite/index.cjs#L29.

await execa(
`node ${require.resolve(
'@redwoodjs/vite/bins/rw-vite-build.mjs',
Expand Down
4 changes: 4 additions & 0 deletions packages/cli/src/commands/devHandler.js
Expand Up @@ -169,6 +169,10 @@ export const handler = async ({
// Written this way to make it easier to read

// 1. default: Vite (SPA)
//
// Disable the new warning in Vite v5 about the CJS build being deprecated
// so that users don't have to see it every time the dev server starts up.
process.env.VITE_CJS_IGNORE_WARNING = 'true'
let webCommand = `yarn cross-env NODE_ENV=development rw-vite-dev ${forward}`

// 2. Vite with SSR
Expand Down
16 changes: 8 additions & 8 deletions packages/project-config/src/__tests__/paths.test.ts
Expand Up @@ -146,7 +146,7 @@ describe('paths', () => {
'web',
'dist',
'server',
'entry.server.js',
'entry.server.mjs',
),
distRouteHooks: path.join(
FIXTURE_BASEDIR,
Expand All @@ -163,7 +163,7 @@ describe('paths', () => {
'web',
'dist',
'server',
'Document.js',
'Document.mjs',
),
distRscEntries: path.join(
FIXTURE_BASEDIR,
Expand Down Expand Up @@ -423,14 +423,14 @@ describe('paths', () => {
'web',
'dist',
'server',
'entry.server.js',
'entry.server.mjs',
),
distDocumentServer: path.join(
FIXTURE_BASEDIR,
'web',
'dist',
'server',
'Document.js',
'Document.mjs',
),
distRouteHooks: path.join(
FIXTURE_BASEDIR,
Expand Down Expand Up @@ -749,14 +749,14 @@ describe('paths', () => {
'web',
'dist',
'server',
'entry.server.js',
'entry.server.mjs',
),
distDocumentServer: path.join(
FIXTURE_BASEDIR,
'web',
'dist',
'server',
'Document.js',
'Document.mjs',
), // this is constructed regardless of presence of src/Document
distRouteHooks: path.join(
FIXTURE_BASEDIR,
Expand Down Expand Up @@ -1026,14 +1026,14 @@ describe('paths', () => {
'web',
'dist',
'server',
'entry.server.js',
'entry.server.mjs',
),
distDocumentServer: path.join(
FIXTURE_BASEDIR,
'web',
'dist',
'server',
'Document.js',
'Document.mjs',
),
distRouteHooks: path.join(
FIXTURE_BASEDIR,
Expand Down
20 changes: 6 additions & 14 deletions packages/project-config/src/paths.ts
Expand Up @@ -130,9 +130,8 @@ const PATH_WEB_DIR_DIST_CLIENT = 'web/dist/client'
const PATH_WEB_DIR_DIST_RSC = 'web/dist/rsc'
const PATH_WEB_DIR_DIST_SERVER = 'web/dist/server'

// Don't specify extension, handled by resolve file
const PATH_WEB_DIR_DIST_SERVER_ENTRY_SERVER = 'web/dist/server/entry.server'
const PATH_WEB_DIR_DIST_DOCUMENT = 'web/dist/server/Document'
const PATH_WEB_DIR_DIST_SERVER_ENTRY_SERVER = 'web/dist/server/entry.server.mjs'
const PATH_WEB_DIR_DIST_DOCUMENT = 'web/dist/server/Document.mjs'

const PATH_WEB_DIR_DIST_SERVER_ROUTEHOOKS = 'web/dist/server/routeHooks'
const PATH_WEB_DIR_DIST_RSC_ENTRIES = 'web/dist/rsc/entries.mjs'
Expand Down Expand Up @@ -248,12 +247,11 @@ export const getPaths = (BASE_DIR: string = getBaseDir()): Paths => {
distRsc: path.join(BASE_DIR, PATH_WEB_DIR_DIST_RSC),
distServer: path.join(BASE_DIR, PATH_WEB_DIR_DIST_SERVER),
// Allow for the possibility of a .mjs file
distEntryServer: mjsOrJs(
path.join(BASE_DIR, PATH_WEB_DIR_DIST_SERVER_ENTRY_SERVER),
),
distDocumentServer: mjsOrJs(
path.join(BASE_DIR, PATH_WEB_DIR_DIST_DOCUMENT),
distEntryServer: path.join(
BASE_DIR,
PATH_WEB_DIR_DIST_SERVER_ENTRY_SERVER,
),
distDocumentServer: path.join(BASE_DIR, PATH_WEB_DIR_DIST_DOCUMENT),
distRouteHooks: path.join(BASE_DIR, PATH_WEB_DIR_DIST_SERVER_ROUTEHOOKS),
distRscEntries: path.join(BASE_DIR, PATH_WEB_DIR_DIST_RSC_ENTRIES),
routeManifest: path.join(BASE_DIR, PATH_WEB_DIR_ROUTE_MANIFEST),
Expand Down Expand Up @@ -433,9 +431,3 @@ export function projectIsEsm() {

return true
}

/** Default to JS path, but if MJS exists, use it instead */
const mjsOrJs = (filePath: string) => {
const mjsPath = resolveFile(filePath, ['.mjs'])
return mjsPath ? mjsPath : filePath + '.js'
}
4 changes: 2 additions & 2 deletions packages/vite/package.json
Expand Up @@ -83,7 +83,7 @@
"isbot": "3.7.1",
"react": "0.0.0-experimental-e5205658f-20230913",
"react-server-dom-webpack": "0.0.0-experimental-e5205658f-20230913",
"vite": "4.5.2",
"vite": "5.1.5",
"vite-plugin-cjs-interop": "2.1.0",
"yargs-parser": "21.1.1"
},
Expand All @@ -94,7 +94,7 @@
"@types/react": "^18.2.55",
"@types/yargs-parser": "21.0.3",
"glob": "10.3.10",
"rollup": "3.29.4",
"rollup": "4.12.1",
"tsx": "4.6.2",
"typescript": "5.3.3",
"vitest": "1.3.1"
Expand Down
6 changes: 3 additions & 3 deletions packages/vite/src/lib/getMergedConfig.ts
Expand Up @@ -118,11 +118,11 @@ export function getMergedConfig(rwConfig: Config, rwPaths: Paths) {
? rwPaths.web.distClient
: rwPaths.web.dist,
emptyOutDir: true,
manifest: !env.ssrBuild ? 'client-build-manifest.json' : undefined,
manifest: !env.isSsrBuild ? 'client-build-manifest.json' : undefined,
// Note that sourcemap can be boolean or 'inline'
sourcemap: !env.ssrBuild && rwConfig.web.sourceMap,
sourcemap: !env.isSsrBuild && rwConfig.web.sourceMap,
rollupOptions: {
input: getRollupInput(!!env.ssrBuild),
input: getRollupInput(!!env.isSsrBuild),
Comment on lines -121 to +125
Copy link
Contributor

@jtoar jtoar Mar 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the breaking changes; ssrBuild was renamed to isSsrBuild. See vitejs/vite#14855

},
},
// @MARK: do not set buildSsrCjsExternalHeuristics here
Expand Down
4 changes: 0 additions & 4 deletions packages/vite/src/rsc/rscBuildForServer.ts
Expand Up @@ -37,10 +37,6 @@ export async function rscBuildForServer(
// TODO (RSC): No redwood-vite plugin, add it in here
const rscServerBuildOutput = await viteBuild({
envFile: false,
legacy: {
// @MARK: for the worker, we're building ESM! (not CJS)
buildSsrCjsExternalHeuristics: false,
},
Comment on lines -40 to -43
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is gone in Vite v5; see vitejs/vite#13816

ssr: {
// Externalize everything except packages with files that have
// 'use client' in them (which are the files in `clientEntryFiles`)
Expand Down
14 changes: 12 additions & 2 deletions packages/vite/src/streaming/createReactStreamingHandler.ts
@@ -1,6 +1,7 @@
import path from 'path'

import { Response } from '@whatwg-node/fetch'
import fs from 'fs-extra'
import isbot from 'isbot'
import type { ViteDevServer } from 'vite'

Expand Down Expand Up @@ -41,9 +42,18 @@ export const createReactStreamingHandler = async (
let fallbackDocumentImport: any

if (isProd) {
entryServerImport = await import(makeFilePath(rwPaths.web.distEntryServer))
const files = await fs.readdir(rwPaths.web.dist, { recursive: true })

console.log({
cwd: process.cwd(),
distEntryServer: rwPaths.web.distEntryServer,
makeFilePathDistEntryServer: makeFilePath(rwPaths.web.distEntryServer),
files,
})

entryServerImport = await import(`file://${rwPaths.web.distEntryServer}`)
fallbackDocumentImport = await import(
makeFilePath(rwPaths.web.distDocumentServer)
`file://${rwPaths.web.distDocumentServer}`
)
}

Expand Down