navigator object breaks projects in v21.0.0 #50269

jim-king-2000 · 2023-10-19T04:23:44Z

Version

21.0.0

Platform

ubuntu

Subsystem

No response

What steps will reproduce the bug?

Open https://github.com/Cache-Hit-Shanghai/jujiuipcappui/actions/runs/6569771442/job/17846176075
Notice the error message:

TypeError: Cannot read properties of undefined (reading 'match')
    at 90370 (/home/runner/work/jujiuipcappui/jujiuipcappui/.next/server/chunks/9650.js:160:29989)
    at __webpack_require__ (/home/runner/work/jujiuipcappui/jujiuipcappui/.next/server/webpack-runtime.js:1:145)
    at 21570 (/home/runner/work/jujiuipcappui/jujiuipcappui/.next/server/chunks/9650.js:160:[18](https://github.com/Cache-Hit-Shanghai/jujiuipcappui/actions/runs/6569771442/job/17846176075#step:8:19)352)
    at __webpack_require__ (/home/runner/work/jujiuipcappui/jujiuipcappui/.next/server/webpack-runtime.js:1:145)
    at 27[20](https://github.com/Cache-Hit-Shanghai/jujiuipcappui/actions/runs/6569771442/job/17846176075#step:8:21)6 (/home/runner/work/jujiuipcappui/jujiuipcappui/.next/server/chunks/9650.js:161:47571)
    at __webpack_require__ (/home/runner/work/jujiuipcappui/jujiuipcappui/.next/server/webpack-runtime.js:1:145)
    at 45131 (/home/runner/work/jujiuipcappui/jujiuipcappui/.next/server/chunks/9650.js:154:116469)
    at __webpack_require__ (/home/runner/work/jujiuipcappui/jujiuipcappui/.next/server/webpack-runtime.js:1:145)
    at 82043 (/home/runner/work/jujiuipcappui/jujiuipcappui/.next/server/chunks/9650.js:154:115774)
    at __webpack_require__ (/home/runner/work/jujiuipcappui/jujiuipcappui/.next/server/webpack-runtime.js:1:145)

Error occurred prerendering page "/en/device/contacts/add". Read more: https://nextjs.org/docs/messages/prerender-error

How often does it reproduce? Is there a required condition?

It reproduces every time. No condition.

What is the expected behavior? Why is that the expected behavior?

Build it successfully. Fall back to v20 it works well.

What do you see instead?

TypeError: Cannot read properties of undefined (reading 'match')
    at 90370 (/home/runner/work/jujiuipcappui/jujiuipcappui/.next/server/chunks/9650.js:160:29989)
    at __webpack_require__ (/home/runner/work/jujiuipcappui/jujiuipcappui/.next/server/webpack-runtime.js:1:145)
    at 21570 (/home/runner/work/jujiuipcappui/jujiuipcappui/.next/server/chunks/9650.js:160:[18](https://github.com/Cache-Hit-Shanghai/jujiuipcappui/actions/runs/6569771442/job/17846176075#step:8:19)352)
    at __webpack_require__ (/home/runner/work/jujiuipcappui/jujiuipcappui/.next/server/webpack-runtime.js:1:145)
    at 27[20](https://github.com/Cache-Hit-Shanghai/jujiuipcappui/actions/runs/6569771442/job/17846176075#step:8:21)6 (/home/runner/work/jujiuipcappui/jujiuipcappui/.next/server/chunks/9650.js:161:47571)
    at __webpack_require__ (/home/runner/work/jujiuipcappui/jujiuipcappui/.next/server/webpack-runtime.js:1:145)
    at 45131 (/home/runner/work/jujiuipcappui/jujiuipcappui/.next/server/chunks/9650.js:154:116469)
    at __webpack_require__ (/home/runner/work/jujiuipcappui/jujiuipcappui/.next/server/webpack-runtime.js:1:145)
    at 82043 (/home/runner/work/jujiuipcappui/jujiuipcappui/.next/server/chunks/9650.js:154:115774)
    at __webpack_require__ (/home/runner/work/jujiuipcappui/jujiuipcappui/.next/server/webpack-runtime.js:1:145)

Error occurred prerendering page "/en/device/contacts/add". Read more: https://nextjs.org/docs/messages/prerender-error

Additional information

It is a regression.

yunseyeong · 2023-10-19T05:15:09Z

i have same problem too,

targos · 2023-10-19T06:26:35Z

This is probably due to #47769

I can reproduce a similar crash with a Next.js app. In my case the problem is with this code (not mine, it's from a dependency):

const userAgent = typeof navigator !== "undefined" ? navigator.userAgent : "";
const commandKeyExists = userAgent.includes("Macintosh") || userAgent.includes("iPad") || userAgent.includes("iPhone");

targos · 2023-10-19T06:32:25Z

/cc @nodejs/tsc I don't know if it was considered before landing the navigator object, but I expect this to affect a lot of front-end code with server-side rendering (or static site generation).
People could previously rely on the fact that if navigator is present, they are in a browser context and can access some properties on it that all browsers implement.

See https://grep.app/search?q=typeof%20navigator%20%21%3D%3D

jim-king-2000 · 2023-10-19T06:59:05Z

#47769 adds navigator to server side context?

Uzlopak · 2023-10-19T08:14:44Z

When we merge PR #43155 it maybe mitigates this issue?

mcollina · 2023-10-19T08:20:46Z

@targos I don't think anybody raised that objection. Given I was skeptical to begin with to add this, I'm also ok for dropping.

anonrig · 2023-10-19T09:54:55Z

@targos because of this risk, we made it a semver major. navigator is in the WHATWG minimum recommended API. we can remove it, but eventually we need to implement/add WHATWG recommendations, don't we?

targos · 2023-10-19T09:59:25Z

because of this risk, we made it a semver major

That's what we always do with new globals. There should be a very good reason to potentially break so many users (and there's not much they can do in this case, since it breaks their dependencies).

eventually we need to implement/add WHATWG recommendations, don't we?

why?

mcollina · 2023-10-19T10:00:20Z

@Ethan-Arrowood @styfle wdyt?

targos · 2023-10-19T10:01:23Z

There is kind of a double-sided problem here:

On one hand, we are adding more and more browser APIs to help with interoperability
On the other hand, this specific addition seems to have the opposite effect: interop is broken

Uzlopak · 2023-10-19T10:12:53Z

IMHO

It was a strategic mistak to add navigator in this minimalistic shape and form. It is not a mvp, as it lacks alot of features.

I assume, that when you green light navigator, we could provide PRs to implement navigator with all necessary flags and options in the next couple of days.

Detecting jsdom in browser-or-node package is interesting: https://github.com/flexdinesh/browser-or-node/blob/master/src/index.js

jim-king-2000 · 2023-10-19T13:12:42Z

So, what is the solution?

anonrig · 2023-10-19T13:14:29Z

I think the problem is people check for browser instead of checking for node.js.

aduh95 · 2023-10-19T13:24:37Z

A first step would probably be to add a CLI flag to disable the global, as we have for fetch and crypto, so users have at least a workaround.

GeoffreyBooth · 2023-10-19T13:43:18Z

Removing the global isn't the solution. Code that checks for it will just need to update. Sure we can also add a flag to disable it, but I'd think they could do so themselves?

node --import 'data:text/javascript,delete globalThis.navigator' app.js

targos · 2023-10-19T13:47:11Z

I wouldn't be so categorical. If the global does more harm than good, removing it, or putting it behind a flag is a reasonable solution.

IMO navigator is not useful enough in its current form to justify the breakage.

marco-ippolito · 2023-10-19T13:54:10Z

I'd say to put it behind a flag, until is mature enough

GeoffreyBooth · 2023-10-19T14:12:44Z

@jim-king-2000 What is the line of code your build is erroring on? Can you provide a minimal reproduction?

jim-king-2000 · 2023-10-19T14:36:28Z

What is the line of code your build is erroring on?

I don't know. It is the code of some library.

Can you provide a minimal reproduction?

Sure. But it would take some time.

GeoffreyBooth · 2023-10-19T16:33:14Z

Well from TypeError: Cannot read properties of undefined (reading 'match') it’s not obvious that this has anything to do with navigator. I think unless we can get more details, ideally a minimal reproduction, we shouldn’t jump to any conclusions.

@targos Perhaps you can open a separate issue for your Next app, if you can pin down the cause for that one.

anonrig · 2023-10-19T16:38:33Z

IMO navigator is not useful enough in its current form to justify the breakage.

@targos I think this should be brought up to WINTERCG. I don't know the main purpose of adding navigator to minimum recommended API but if we're going to remove it as a global, we should at least inform them about this.

GeoffreyBooth · 2023-10-19T16:54:28Z

if we’re going to remove it as a global

I’m opposed to removing it. I don’t think we should assume that this is our course of action, or reach out to anyone with that assumption until we decide that this is the direction we want to go.

tniessen · 2023-10-19T22:25:40Z

I don't think anybody raised that objection. Given I was skeptical to begin with to add this, I'm also ok for dropping.

@mcollina I explicitly raised this specific concern, e.g., in #39540 (comment). I also was highly skeptical of adding navigator, as I expressed in various discussions. However, I was unaware that navigator had indeed been added until Node.js 21 was released.

I think the problem is people check for browser instead of checking for node.js.

@anonrig I think that's somewhat reasonable. The only relevant specification is the HTML Standard, which Node.js does not claim to implement (besides a small subset of APIs), nor are we obligated to implement any of it. Section 8.9 of the specification defines a navigator as a "user agent (the client)" and later in that section refers specifically to "web browsers". Historically, the word "navigator" was used for early web browsers (i.e., Netscape). Indeed, a spec-compliant implementation of NavigatorID, which includes navigator.userAgent etc., must set navigator.appName to Netscape. Due to its nature, the Navigator interface is also not exposed on any globalThis, but specifically on Window. The navigator.onLine property suggested in #50224 is even listed under "Browser state".

Therefore, I think it's fair for people to assume that software that defines navigator is, well, a navigator (or web browser).

GeoffreyBooth · 2023-10-19T22:45:12Z

The relevant spec for us is https://common-min-api.proposal.wintercg.org/#requirements-for-navigatoruseragent.

Deno implements navigator: https://deno.land/[email protected]?s=Navigator

deno
Deno 1.37.1
exit using ctrl+d, ctrl+c, or close()
REPL is running with all permissions allowed.
To specify permissions, run `deno repl` with allow flags.
> navigator
Navigator {}
> navigator.userAgent
"Deno/1.37.1"
> navigator.hardwareConcurrency
16
> navigator.language
"en-US"
> navigator.languages
[ "en-US" ]

And so does Bun:

bun repl
Welcome to Bun v1.0.6
Type ".help" for more information.
[!] Please note that the REPL implementation is still experimental!
    Don't consider it to be representative of the stability or behavior of Bun overall.
> navigator
Object [Navigator] {
  userAgent: 'Bun/1.0.6',
  platform: 'MacIntel',
  hardwareConcurrency: 16
}

tniessen · 2023-10-19T23:07:59Z

The relevant spec for us is https://common-min-api.proposal.wintercg.org/#requirements-for-navigatoruseragent.

Sure, but the WinterCG spec again delegates to the HTML standard (see "Terms defined by reference") for the term "navigator" (i.e., web browser). I am not saying that WinterCG is right or wrong, nor am I saying that following the WinterCG's suggestions is right or wrong. All I am saying is that users are somewhat justified in assuming that a runtime that implements navigator is a navigator, and that breakage was expected (and apparently accepted by some).

GeoffreyBooth · 2023-10-19T23:22:28Z

Semver majors have breaking changes. That’s what they’re for.

In this case adding navigator brings us into compatibility alignment with both Deno and Bun and browsers. Adding it was the right call, and libraries will just need to update.

We also don’t need to create a flag to disable it. That’s already possible today:

$ node --print 'navigator'
Navigator {}

$ node --import 'data:text/javascript,delete globalThis.navigator' --print 'navigator'
[eval]:1
navigator
^

ReferenceError: navigator is not defined

We could perhaps add a mention of --import 'data:text/javascript,delete globalThis.navigator' to the docs section for navigator, to advise users how to remove this global until their dependencies update.

GeoffreyBooth · 2023-10-24T16:28:40Z

@jim-king-2000 Node 21.1.0 is out now. Does it fix your issue?

jim-king-2000 · 2023-10-25T09:18:16Z

@jim-king-2000 Node 21.1.0 is out now. Does it fix your issue?

Yes. Thanks for the quick fix.

jim-king-2000 · 2023-10-25T09:19:29Z

And I think we could close this issue.

zm-cttae · 2023-10-25T10:17:06Z

oven-sh/bun#4588

Uzlopak · 2023-10-25T10:20:52Z

I have platform implementation in my local node repo. Will provide in an hour a PR. But if tsc decides to delete navigator..

Uzlopak · 2023-10-25T11:53:50Z

@mcollina
I guess we can unpin this issue also?

mhdawson · 2023-10-25T15:10:37Z

Discussed in the TSC meeting, agreed it no longer needs to be on the agenda.

KhafraDev · 2023-10-25T15:52:35Z

was there a consensus on removing navigator, or just that node is no longer breaking things with the addition of navigator.userAgent?

Uzlopak · 2023-10-25T15:56:38Z

It was implicitly accepted. See the video to the livestream. It was not explicitly discussed about removing it.

tniessen · 2023-10-25T15:56:52Z

As far as I can tell from the meeting notes, there was no consensus either way. Only this particular issue is considered to have been resolved.

Uzlopak · 2023-10-25T16:01:59Z

From the video i can say that the tsc members agreed on that we should have added userAgent first because it is the most popular attribute and this was a mistake. Now we added it and also there were some citgm changes to detect such a change regarding browser compatibility in the future better.

KhafraDev · 2023-10-25T16:06:49Z

I still don't see the value in having navigator but I agree that navigator should have included userAgent when it was added. I wouldn't call that an agreement that node should keep it.

GeoffreyBooth · 2023-10-25T16:21:59Z

Any contributor can open a PR to remove navigator. That’s not yet happened as far as I’m aware, so that wasn’t on the agenda for the TSC to discuss. You can make whatever inferences you want from the discussion that did occur.

If a PR to remove navigator gets opened and then gets blocked (and to be transparent, I would block it) then anyone can request that the TSC override the block. Then the TSC would debate that issue and reach a conclusion.

KhafraDev · 2023-10-25T16:44:46Z

The issue there is that it's much more effort to revert the change than it is to keep it, where I have the time to make comments on issues, but I don't have the time to maintain a PR that, admittedly, would be blocked. Kinda meta, but how would removing it work - at least one tsc member believes it should be kept, which means there'd be a stalemate?

GeoffreyBooth · 2023-10-25T16:47:05Z

how would removing it work - at least one tsc member believes it should be kept, which means there’d be a stalemate?

If a question for the TSC is discussed and there isn’t consensus (unanimous agreement) then it goes to a simple majority vote of the TSC.

zm-cttae · 2023-10-28T10:39:24Z

Just to confirm - is this an up-to-date link for the Navigator Web API?

https://html.spec.whatwg.org/multipage/system-state.html#the-navigator-object

It's the link I posted in #50385 too.

fabb · 2024-11-01T11:26:29Z

a lot of projects use typescript and the global navigator object type currently points to lib.dom.d.ts. maybe that can be updated to at least cause compile errors?

styfle mentioned this issue Oct 19, 2023

chore(test): run all tests against node@21 vercel/next.js#57057

Draft

tniessen added the experimental Issues and PRs related to experimental features. label Oct 19, 2023

tniessen changed the title ~~v21.0.0 fails to build my project~~ navigator object breaks projects in v21.0.0 Oct 19, 2023

tniessen added the v21.x label Oct 19, 2023

mhdawson mentioned this issue Oct 23, 2023

Node.js Technical Steering Committee (TSC) Meeting 2023-10-25 nodejs/TSC#1457

Closed

mcollina closed this as completed Oct 25, 2023

tniessen mentioned this issue Oct 25, 2023

lib: add navigator.platform #50385

Merged

mhdawson removed the tsc-agenda Issues and PRs to discuss during the meetings of the TSC. label Oct 25, 2023

KhafraDev unpinned this issue Oct 25, 2023

GeoffreyBooth mentioned this issue Oct 26, 2023

lib: hide navigator behind a flag #50412

Closed

bjoerge mentioned this issue Nov 14, 2023

Error running sanity studio locally right after project initialization sanity-io/sanity#5183

Closed

GeoffreyBooth mentioned this issue Dec 8, 2023

Navigator APIs wintercg/proposal-minimum-common-api#61

Closed

Ethan-Arrowood mentioned this issue Jan 9, 2024

Support pnpm; add Next.js nodejs/citgm#1017

Open

vladimirrostok mentioned this issue Mar 14, 2024

DeFi wallet login buton breaks the Next.js build multiversx/mx-template-dapp-nextjs#52

Closed

valgaze mentioned this issue Apr 6, 2024

Webex SDK 3.0.0 not compatible with any version of NodeJS webex/webex-js-sdk#3510

Open

This was referenced Nov 1, 2024

Type of global navigator object incorrect since node.js 21 microsoft/TypeScript#60387

Closed

Improve dom types for server side rendering environments microsoft/TypeScript#53971

Open

navigator object breaks projects in v21.0.0 #50269

navigator object breaks projects in v21.0.0 #50269

Comments

jim-king-2000 commented Oct 19, 2023 • edited Loading

Version

Platform

Subsystem

What steps will reproduce the bug?

How often does it reproduce? Is there a required condition?

What is the expected behavior? Why is that the expected behavior?

What do you see instead?

Additional information

yunseyeong commented Oct 19, 2023

targos commented Oct 19, 2023 • edited Loading

targos commented Oct 19, 2023

jim-king-2000 commented Oct 19, 2023 • edited Loading

Uzlopak commented Oct 19, 2023

mcollina commented Oct 19, 2023

anonrig commented Oct 19, 2023

targos commented Oct 19, 2023

mcollina commented Oct 19, 2023

targos commented Oct 19, 2023

Uzlopak commented Oct 19, 2023

jim-king-2000 commented Oct 19, 2023

anonrig commented Oct 19, 2023

aduh95 commented Oct 19, 2023

GeoffreyBooth commented Oct 19, 2023

targos commented Oct 19, 2023

marco-ippolito commented Oct 19, 2023

GeoffreyBooth commented Oct 19, 2023

jim-king-2000 commented Oct 19, 2023 • edited Loading

GeoffreyBooth commented Oct 19, 2023

anonrig commented Oct 19, 2023

GeoffreyBooth commented Oct 19, 2023

tniessen commented Oct 19, 2023

GeoffreyBooth commented Oct 19, 2023 • edited Loading

tniessen commented Oct 19, 2023

GeoffreyBooth commented Oct 19, 2023

GeoffreyBooth commented Oct 24, 2023

jim-king-2000 commented Oct 25, 2023

jim-king-2000 commented Oct 25, 2023

zm-cttae commented Oct 25, 2023

Uzlopak commented Oct 25, 2023

Uzlopak commented Oct 25, 2023

mhdawson commented Oct 25, 2023

KhafraDev commented Oct 25, 2023

Uzlopak commented Oct 25, 2023

tniessen commented Oct 25, 2023

Uzlopak commented Oct 25, 2023

KhafraDev commented Oct 25, 2023

GeoffreyBooth commented Oct 25, 2023 • edited Loading

KhafraDev commented Oct 25, 2023

GeoffreyBooth commented Oct 25, 2023

zm-cttae commented Oct 28, 2023 • edited Loading

fabb commented Nov 1, 2024

jim-king-2000 commented Oct 19, 2023 •

edited

Loading

targos commented Oct 19, 2023 •

edited

Loading

jim-king-2000 commented Oct 19, 2023 •

edited

Loading

jim-king-2000 commented Oct 19, 2023 •

edited

Loading

GeoffreyBooth commented Oct 19, 2023 •

edited

Loading

GeoffreyBooth commented Oct 25, 2023 •

edited

Loading

zm-cttae commented Oct 28, 2023 •

edited

Loading