GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — lib: add navigator.language and navigator.languages by Uzlopak · Pull Request #50303 · nodejs/node

Conversation

@Uzlopak

@Uzlopak Uzlopak commented Oct 20, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

This adds navigator.language and navigator.languages based on the capabilities of ICU.

@anonrig

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss, H4ad, and gjivaros reacted with thumbs up emoji
All reactions
  • 👍 3 reactions
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. i18n-api Issues and PRs related to Node.js internationalization support. needs-ci PRs that need a full CI run. labels Oct 20, 2023
tniessen
tniessen reviewed Oct 20, 2023
View reviewed changes
Comment thread doc/api/globals.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread doc/api/globals.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
anonrig
anonrig reviewed Oct 20, 2023
View reviewed changes
Comment thread lib/internal/navigator.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread lib/internal/navigator.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread lib/internal/navigator.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/node_i18n.cc Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/node_i18n.cc Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread test/parallel/test-navigator.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread doc/api/globals.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
@GeoffreyBooth GeoffreyBooth added the web-standards Issues and PRs related to web-platform APIs and standards compliance. label Oct 26, 2023
@GeoffreyBooth

GeoffreyBooth commented Oct 26, 2023

Copy link
Copy Markdown
Member

Let’s please ping @nodejs/web-standards for all issues and PRs related to navigator.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
panva
panva previously requested changes Oct 26, 2023
View reviewed changes

@panva panva left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This doesn't seem entirely useful given it's based on the capabilities of ICU which for most is bundled.

Consider this a non-blocking -1 that you may dismiss given there's demand for this behaviour.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread doc/api/globals.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
@Uzlopak
Uzlopak force-pushed the navigatorLanguageS branch from f82d7a3 to 5bdd487 Compare October 29, 2023 02:22
@GeoffreyBooth

GeoffreyBooth commented Oct 29, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

This doesn't seem entirely useful given it's based on the capabilities of ICU which for most is bundled.

Consider this a non-blocking -1 that you may dismiss given there’s demand for this behaviour.

@panva can you explain? What do you mean by “given it’s based on the capabilities of ICU which for most is bundled”?

The reason to add these properties, like most of the navigator properties, is to enable code that was written for browsers to run without erroring in Node. All of the navigator properties are duplicative of existing Node APIs that provide the same information; that’s irrelevant, because the point is to support code that isn’t written for Node.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Uzlopak
Uzlopak force-pushed the navigatorLanguageS branch 2 times, most recently from 332aa0a to 74ac025 Compare October 29, 2023 02:35
@Uzlopak

Uzlopak commented Oct 29, 2023

Copy link
Copy Markdown
Contributor Author

I thought about this PR and simplified it.

Can you set the default locale of icu in node? If yes, than this PR is still wrong, as it should use that value instead.
But if you cant set the default locale of icu, than this PR should be correct.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@GeoffreyBooth

GeoffreyBooth commented Oct 29, 2023

Copy link
Copy Markdown
Member

Can you set the default locale of icu in node? If yes, than this PR is still wrong, as it should use that value instead.

Yes, you can:

LC_ALL=en_US node --print 'new Date().toLocaleString()'
10/28/2023, 8:13:49 PM

LC_ALL=en_UK node --print 'new Date().toLocaleString()'
10/28/2023, 20:13:55

I don’t know if LC_ALL is the only way to set it, but it’s at least one way. You can use Intl.DateTimeFormat().resolvedOptions().locale to see what Node is using:

LC_ALL=en_US node --print 'Intl.DateTimeFormat().resolvedOptions().locale'
en-US

LC_ALL=en_UK node --print 'Intl.DateTimeFormat().resolvedOptions().locale'
en-UK
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@panva
panva dismissed their stale review October 29, 2023 08:08

It was non-blocking in the first place. If the new approach just returns Intl.DateTimeFormat().resolvedOptions().locale it's fine.

@Uzlopak
Uzlopak force-pushed the navigatorLanguageS branch 3 times, most recently from ec11b6d to f0ab0c0 Compare October 29, 2023 13:01
@Uzlopak

Uzlopak commented Oct 29, 2023

Copy link
Copy Markdown
Contributor Author

@GeoffreyBooth

Nice. LOL, you dont know how much i searched around to find this retrieval opportunity.

Falling back to 'en-US' if Intl does not exist. I guess. how can I configure make to build with full intl?

I oriented the functionality based on the behaviour of Chrome and Firefox. So if you use defineProperty to overwrite navigator.language, then languages wont be changed, like in the browser. Also the Array of navigator.languages is frozen, like it is in chrome and firefox.

Can someone please add the 'author-ready' label and remove the 'c++' and 'i18n-api' labels
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
GeoffreyBooth
GeoffreyBooth reviewed Oct 29, 2023
View reviewed changes

@GeoffreyBooth GeoffreyBooth left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Let's add a test that uses spawnPromisified to start a Node process with an LC_ALL of something non-default, and see that it carries through to this new property.

We should also mention in the docs that this environment variable is one way to set Node's default language.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@GeoffreyBooth

GeoffreyBooth commented Oct 29, 2023

Copy link
Copy Markdown
Member

how can I configure make to build with full intl?

See https://github.com/nodejs/node/blob/65087c04866dad805554102592754f1262bfb3b2/doc/api/intl.md
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ljharb
ljharb reviewed Oct 29, 2023
View reviewed changes
Comment thread lib/internal/navigator.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
jasnell
jasnell reviewed Oct 29, 2023
View reviewed changes
Comment thread doc/api/globals.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
jasnell
jasnell reviewed Oct 29, 2023
View reviewed changes
Comment thread lib/internal/navigator.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
@Uzlopak
Uzlopak force-pushed the navigatorLanguageS branch 2 times, most recently from fed4162 to 701237a Compare October 30, 2023 00:35
@Uzlopak

Uzlopak commented Oct 30, 2023

Copy link
Copy Markdown
Contributor Author

@panva
@jasnell
@GeoffreyBooth
@anonrig

PTAL

And if somebody approves, please add then the author-ready label :)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
GeoffreyBooth
GeoffreyBooth reviewed Oct 30, 2023
View reviewed changes
Comment thread doc/api/globals.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread doc/api/globals.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread doc/api/globals.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread doc/api/globals.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
@Uzlopak Uzlopak closed this Oct 30, 2023
@Uzlopak
Uzlopak force-pushed the navigatorLanguageS branch from 701237a to 14af167 Compare October 30, 2023 08:00
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Nov 4, 2023
@nodejs-github-bot

nodejs-github-bot commented Nov 4, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/55442/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Uzlopak
Uzlopak requested review from jasnell, ljharb, panva, targos and tniessen November 4, 2023 16:46
@GeoffreyBooth GeoffreyBooth added commit-queue PRs queued for automated landing through the Commit Queue. commit-queue-squash PRs the Commit Queue should land as one squashed commit. labels Nov 4, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Nov 4, 2023
@nodejs-github-bot
nodejs-github-bot merged commit 77b0595 into nodejs:main Nov 4, 2023
@nodejs-github-bot

nodejs-github-bot commented Nov 4, 2023

Copy link
Copy Markdown
Collaborator

Landed in 77b0595
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot mentioned this pull request Nov 5, 2023
CI Reliability 2023-11-05 nodejs/reliability#708
Open
13 tasks
@Uzlopak
Uzlopak deleted the navigatorLanguageS branch November 5, 2023 12:02
This was referenced Nov 6, 2023
CI Reliability 2023-11-06 nodejs/reliability#709
Open
CI Reliability 2023-11-07 nodejs/reliability#710
Open
CI Reliability 2023-11-08 nodejs/reliability#711
Open
targos pushed a commit that referenced this pull request Nov 11, 2023
@Uzlopak @targos
lib: add navigator.language & navigator.languages
80f5bee 
PR-URL: #50303
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
@targos targos mentioned this pull request Nov 12, 2023
v21.2.0 release proposal #50681
Merged
@targos targos added the semver-minor PRs that contain new features and should be released in the next minor version. label Nov 13, 2023
@targos

targos commented Nov 13, 2023

Copy link
Copy Markdown
Member

This is semver-minor. Collaborators, please think about adding the label in future pull requests. It will make the work of releasers easier.
richardlau reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
targos added a commit that referenced this pull request Nov 13, 2023
@targos
2023-11-14, Version 21.2.0 (Current)
0f82899 
Notable changes:

doc:
  * add MrJithil to collaborators (Jithil P Ponnan) #50666
  * add Ethan-Arrowood as a collaborator (Ethan Arrowood) #50393
esm:
  * (SEMVER-MINOR) add import.meta.dirname and import.meta.filename (James Sumners) #48740
fs:
  * add stacktrace to fs/promises (翠 / green) #49849
lib:
  * (SEMVER-MINOR) add `--no-experimental-global-navigator` CLI flag (Antoine du Hamel) #50562
  * (SEMVER-MINOR) add navigator.language & navigator.languages (Aras Abbasi) #50303
  * (SEMVER-MINOR) add navigator.platform (Aras Abbasi) #50385
stream:
  * (SEMVER-MINOR) add support for `deflate-raw` format to webstreams compression (Damian Krzeminski) #50097
  * use Array for Readable buffer (Robert Nagy) #50341
  * optimize creation (Robert Nagy) #50337
test_runner:
  * (SEMVER-MINOR) adds built in lcov reporter (Phil Nash) #50018
  * (SEMVER-MINOR) add Date to the supported mock APIs (Lucas Santos) #48638
test_runner, cli:
  * (SEMVER-MINOR) add --test-timeout flag (Shubham Pandey) #50443

PR-URL: #50681
targos pushed a commit that referenced this pull request Nov 14, 2023
@Uzlopak @targos
lib: add navigator.language & navigator.languages
03c730b 
PR-URL: #50303
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
targos added a commit that referenced this pull request Nov 14, 2023
@targos
2023-11-14, Version 21.2.0 (Current)
13b266f 
Notable changes:

doc:
  * add MrJithil to collaborators (Jithil P Ponnan) #50666
  * add Ethan-Arrowood as a collaborator (Ethan Arrowood) #50393
esm:
  * (SEMVER-MINOR) add import.meta.dirname and import.meta.filename (James Sumners) #48740
fs:
  * add stacktrace to fs/promises (翠 / green) #49849
lib:
  * (SEMVER-MINOR) add `--no-experimental-global-navigator` CLI flag (Antoine du Hamel) #50562
  * (SEMVER-MINOR) add navigator.language & navigator.languages (Aras Abbasi) #50303
  * (SEMVER-MINOR) add navigator.platform (Aras Abbasi) #50385
stream:
  * (SEMVER-MINOR) add support for `deflate-raw` format to webstreams compression (Damian Krzeminski) #50097
  * use Array for Readable buffer (Robert Nagy) #50341
  * optimize creation (Robert Nagy) #50337
test_runner:
  * (SEMVER-MINOR) adds built in lcov reporter (Phil Nash) #50018
  * (SEMVER-MINOR) add Date to the supported mock APIs (Lucas Santos) #48638
test_runner, cli:
  * (SEMVER-MINOR) add --test-timeout flag (Shubham Pandey) #50443

PR-URL: #50681
targos added a commit that referenced this pull request Nov 14, 2023
@targos
2023-11-14, Version 21.2.0 (Current)
02db7fc 
Notable changes:

doc:
  * add MrJithil to collaborators (Jithil P Ponnan) #50666
  * add Ethan-Arrowood as a collaborator (Ethan Arrowood) #50393
esm:
  * (SEMVER-MINOR) add import.meta.dirname and import.meta.filename (James Sumners) #48740
fs:
  * add stacktrace to fs/promises (翠 / green) #49849
lib:
  * (SEMVER-MINOR) add `--no-experimental-global-navigator` CLI flag (Antoine du Hamel) #50562
  * (SEMVER-MINOR) add navigator.language & navigator.languages (Aras Abbasi) #50303
  * (SEMVER-MINOR) add navigator.platform (Aras Abbasi) #50385
stream:
  * (SEMVER-MINOR) add support for `deflate-raw` format to webstreams compression (Damian Krzeminski) #50097
  * use Array for Readable buffer (Robert Nagy) #50341
  * optimize creation (Robert Nagy) #50337
test_runner:
  * (SEMVER-MINOR) adds built in lcov reporter (Phil Nash) #50018
  * (SEMVER-MINOR) add Date to the supported mock APIs (Lucas Santos) #48638
test_runner, cli:
  * (SEMVER-MINOR) add --test-timeout flag (Shubham Pandey) #50443

PR-URL: #50681
@richardlau richardlau added the dont-land-on-v20.x label Mar 25, 2024
@skyclouds2001 skyclouds2001 mentioned this pull request Nov 7, 2024
Add nodejs data for Navigator mdn/browser-compat-data#24988
Merged
@coderabbitai coderabbitai Bot mentioned this pull request Jul 26, 2026
fix(globals): complete Node parity PerryTS/perry#6853
Merged
7 of 8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@GeoffreyBooth GeoffreyBooth GeoffreyBooth approved these changes
@anonrig anonrig anonrig approved these changes
@panva panva Awaiting requested review from panva
@jasnell jasnell Awaiting requested review from jasnell
@ljharb ljharb Awaiting requested review from ljharb
@targos targos Awaiting requested review from targos
@tniessen tniessen Awaiting requested review from tniessen

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. commit-queue-squash PRs the Commit Queue should land as one squashed commit. i18n-api Issues and PRs related to Node.js internationalization support. needs-ci PRs that need a full CI run. semver-minor PRs that contain new features and should be released in the next minor version. web-standards Issues and PRs related to web-platform APIs and standards compliance.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

10 participants

@Uzlopak @GeoffreyBooth @nodejs-github-bot @targos @ljharb @panva @jasnell @anonrig @tniessen @richardlau
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.