Skip to content

Conversation

@lobsterkatie
Copy link
Member

This makes a few changes to the isBuild utility function in the nextjs SDK in order to make its operation less brittle. Currently, it relies on being called at the top level of index.server.ts, which ensures that it runs early enough in the build process to be executed in the main thread, before operation has been split out into child processes. (This is important because child processes don't conveniently have the word 'build' in their invocations the way the main thread (invoked by next build) does, removing one of isBuild's clues about what phase it's in.) It then sets an environment variable as a clue to future calls to itself, so that, child processes or not, it knows the correct phase. This means, however, that if it stops being used at index.server.ts's top level, and doesn't get called until after the process split, it won't have any way to know whether it's in the build phase or not.

This makes two changes to guard against those possibilities:

  • To ensure that it's always called as early as possible in the build, it is now run independently of any use of its return value, instead storing that value in a constant, which is then used anywhere it's needed.
  • To provide a backup indicator of the current phase, it now also checks the next-provided NEXT_PHASE environment variable. (The reason it doesn't simply rely on this variable to begin with is that it's only set partway through the build process.)

As a result of these changes, two other changes are included here:

  • The export of isBuild is deprecated in favor of the computed constant. (Since they're equivalent, no reason to keep exporting them both.)
  • The local constant isVercel has been renamed IS_VERCEL, to match IS_BUILD. (Yes, I'm that kind of OCD.)

Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

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

Capitalization 😎

*/
const deprecatedIsBuild = (): boolean => isBuild();
// eslint-disable-next-line deprecation/deprecation
export { deprecatedIsBuild as isBuild };
Copy link
Member

Choose a reason for hiding this comment

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

l: ehhh do we have to deprecate this? Is anyone even using this export?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have no idea, but it seems to me we don't lose much by deprecating it.

@lobsterkatie lobsterkatie merged commit 2b8237c into master Oct 4, 2022
@lobsterkatie lobsterkatie deleted the kmclb-nextjs-make-isBuild-more-robust branch October 4, 2022 23:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants