Open icyJoseph opened 2 weeks ago
Allow CI Workflow Run
Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer
Thank you. Please add a test here: test/integration/env-config/test/index.test.js
Ideally split the PR in two commits: one commit with just the test asserting the current behavior and another commit with the fix+updated test assertions.
Yeah I tried to first write a test, but I failed to find that file, but made the fix anyway. Thanks for that! I'll work on a test to prevent further regressions.
While setting up a test, I also got to understand the bug a bit better. The problem is that even though the HTML source has the expected "empty string", it is during hydration, that it is getting replaced by undefined
~ I will continue investigating tomorrow.
Edit: Undoing the first commit changes, using the webdriver, I did manage to create tests that fail, because, undefined is found after hydration. Re-applying the commit fixes the issue. Also tested manually by running an app with my local build.
Edit2: This issue was creating a hydration error as well. Though, I guess the root issue is that people are using the NEXT_PUBLIC
env vars, directly in the JSX, with interpolations.
Commit: 233e7acd90078540895bbcc2ec98c7924c62d5c5
pnpm test test/integration/not-found-revalidate/test/index.test.js
Read more about building and testing Next.js in contributing.md.
TURBOPACK=1 pnpm test-start test/e2e/app-dir/app-prefetch/prefetching.test.ts
(turbopack)
_rsc
query based on Next-Url
Read more about building and testing Next.js in contributing.md.
TURBOPACK=1 pnpm test-start test/e2e/app-dir/ppr-full/ppr-full.test.ts
(turbopack)
Read more about building and testing Next.js in contributing.md.
TURBOPACK=1 pnpm test-start test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts
(turbopack)
Read more about building and testing Next.js in contributing.md.
__NEXT_EXPERIMENTAL_PPR=true pnpm test-dev test/e2e/react-compiler/react-compiler.test.ts
(PPR)
Read more about building and testing Next.js in contributing.md.
pnpm test-start test/e2e/app-dir/parallel-routes-and-interception/parallel-routes-and-interception.test.ts
children
parallel slotRead more about building and testing Next.js in contributing.md.
pnpm test-start test/production/prerender-prefetch/index.test.ts
Read more about building and testing Next.js in contributing.md.
pnpm test-dev test/e2e/app-dir/actions/app-action.test.ts
Read more about building and testing Next.js in contributing.md.
__NEXT_EXPERIMENTAL_PPR=true pnpm test-start test/e2e/prerender.test.ts
(PPR)
Read more about building and testing Next.js in contributing.md.
Commit: 233e7acd90078540895bbcc2ec98c7924c62d5c5
What?
Fixes #64832
Having empty strings in
.env
, and family of files, causes the variable to be assigned toundefined
, rather than an empty string.Why?
The issue breaks behavior between canaries
How?
The value read from
process.env[key]
, isstring | undefined
, and we need to narrow down tostring
, but the current check also filters out''
. A quick way to fix that is tovalue != null
~Fixes #64832
~Not sure if there's anywhere, where this is tested in the codebase~
The root issue here is that the value before hydration is
''
, but during hydrationundefined
is used, and using something like, template string interpolations,${NEXT_PUBLIC_EMPTY_ENV_VAR}
, would cause a hydration error. That's what is happening in #64832. For clarity's sake, the error would be that the HTML contained an empty string, but hydration has"undefined"
string cast.