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

"Intl.DateTimeFormat().resolvedOptions().locale" returns different values in extension host to VS Code process #85675

Closed
DanTup opened this issue Nov 27, 2019 · 10 comments · Fixed by #141095
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug confirmed Issue has been confirmed by VS Code Team member electron Issues and items related to Electron macos Issues with VS Code on MAC/OS X upstream Issue identified as 'upstream' component related (exists outside of VS Code) wont-fix

Comments

@DanTup
Copy link
Contributor

DanTup commented Nov 27, 2019

I'm using MacBook with its system language set to en-GB. I launched VS Code (from the dock, not the terminal) and if I run:

Intl.DateTimeFormat().resolvedOptions().locale

in the Chrome dev tools console, I see this:

Screenshot 2019-11-27 at 9 23 00 am

However, in an extension's activate function if I do this:

console.log(Intl.DateTimeFormat().resolvedOptions().locale);

Then I see this:

Screenshot 2019-11-27 at 9 23 44 am

I need to get the system language in order to set the LANG env variable for spawned processes because CocoaPods crashes if LANG is not set to something-UTF-8, and there's no way to set it without a language part so I need to get at the system language.

@alexdima
Copy link
Member

alexdima commented Dec 5, 2019

I don't think we have any code to reset/remove things. @deepak1556 Do you have any ideas? (the extension host is using ELECTRON_RUN_AS_NODE)

@DanTup
Copy link
Contributor Author

DanTup commented Dec 6, 2019

FWIW we've worked around this in Flutter. It still seems weird these don't line up and it might be useful for the extension host to have access to it, but it's not pressing for me anymore. Thanks!

@deepak1556
Copy link
Collaborator

I can confirm this issue,

@alexandrudima here are my observations.

  1. Renderer process prints the proper locale
  2. Extension host reports incorrect locale
  3. Running the Code executable from command line with export ELECTRON_RUN_AS_NODE=1 set code -e 'console.log(Intl.DateTimeFormat().resolvedOptions().locale)' prints the correct locale.

To make things more concrete, v8 and icu are shared between node and electron so behavior being different between these two processes are highly unlikely unless we are intervening.

@deepak1556 deepak1556 added bug Issue identified by VS Code Team member as probable bug confirmed Issue has been confirmed by VS Code Team member extension-host Extension host issues labels Dec 7, 2019
@deepak1556
Copy link
Collaborator

Okay I have narrowed down the issue, it seems to be a problem with child_process module that is messing with the locale. Tried the following sample

const { spawn } = require('child_process');
const locale = spawn("/Users/demohan/Downloads/electron-v6.1.5-darwin-x64/Electron.app/Contents/MacOS/Electron", ['-e', 'console.log(Intl.DateTimeFormat().resolvedOptions().locale)'], {env: {
	'ELECTRON_RUN_AS_NODE': 1
}});

locale.stdout.on('data', (data : any) => {
  console.log(`stdout: ${data}`);
});

locale.stderr.on('data', (data : any) => {
  console.error(`stderr: ${data}`);
});

prints en-US while running on the command line prints the proper system locale used at the moment, in my case de

@deepak1556 deepak1556 added electron Issues and items related to Electron upstream Issue identified as 'upstream' component related (exists outside of VS Code) labels Dec 7, 2019
@alexdima alexdima removed their assignment Feb 28, 2020
@alexdima alexdima removed the extension-host Extension host issues label Oct 15, 2021
@rzhao271 rzhao271 added this to the Backlog milestone Nov 15, 2021
@rzhao271 rzhao271 modified the milestones: Backlog, January 2022 Dec 9, 2021
@rzhao271 rzhao271 added the macos Issues with VS Code on MAC/OS X label Dec 16, 2021
@rzhao271
Copy link
Contributor

Confirmed the issue still occurs on the latest Insiders on Big Sur.

@rzhao271
Copy link
Contributor

rzhao271 commented Dec 30, 2021

I copied the code from #85675 (comment) to a file main.js (with a different Electron spawn path) and did the following:

  1. I added a line console.log(Intl.DateTimeFormat().resolvedOptions().locale) at the end.
  2. In the terminal, I ran export LANG='fr_CA.UTF-8'
  3. 🐛 node main.js results in fr-CA as the locale, but en-US as the spawned locale.
  4. I changed the spawn call to spawn node directly instead.
  5. node main.js results in both locales to be fr-CA.

Keeping the Electron label on this issue.

@rzhao271
Copy link
Contributor

rzhao271 commented Dec 30, 2021

Electron actually reports the locale properly if one changes the env parameter above to the following:

{
    ...process.env,
    'ELECTRON_RUN_AS_NODE': 1
}

Even if we were to spawn just node, I noticed that providing env: {} for the options causes the locale mismatch, too, so the first issue is that the problem isn't in Electron, but has to do with whether we are explicitly passing in process.env or not.

A second issue is the user must set the LANG environment variable in order for process.env to pick up the correct locale. I set my macOS display language to French, and node still displayed en-US as the locale until I explicitly changed the LANG environment variable.

For the first issue, we already use process.env at

Therefore, we aren't forgetting to add process.env.LANG, but rather, process.env.LANG just isn't there by the time that code runs. I confirmed that explicitly adding LANG as an env variable in the launch config to run a custom extension containing the console log line from #85675 (comment) also fixes the issue.

@rzhao271
Copy link
Contributor

rzhao271 commented Jan 13, 2022

So I first thought the issue was as-designed, because my minimal repro was a bit too minimal, but it's actually a more generic upstream issue.
When spawning processes, the child process somehow loses the locale information.

Minimal repro:

main.js

const { app, BrowserWindow } = require('electron');
const cp = require('child_process');

// If we ask for the locale before the app is ready, we actually get en-US here, too.
app.whenReady().then(() => {
    console.log(Intl.DateTimeFormat().resolvedOptions().locale);
    const proc = cp.fork(__dirname + '/child.js');
    proc.on('exit', () => app.quit());
})

child.js

console.log(Intl.DateTimeFormat().resolvedOptions().locale);

Output on macOS with the language set to Spanish (Latin America):

es-MX
en-US

@rzhao271
Copy link
Contributor

In the past week, I learned that

  • If one fetches the locale too early in the main process, before the app is ready, then the incorrect locale is returned
  • The entry point for running electron as node differs from the entry point for electron GUI apps
  • The former entry point doesn't call l10n_util::OverrideLocaleWithCocoaLocale, whereas the latter one does. That function, specific to macOS, allows one to fetch the correct locale in the main process after the app is ready
  • When running electron as node, we want the behaviour to be similar to running node itself, so patching electron as node to override the locale might not be the best idea (hence the closed upstream PR)

My plan now is to fetch the locale before spawning the extension host, and pass the locale in as an environment variable. I'll also merge it in next iteration to get more testing.

rzhao271 added a commit that referenced this issue Feb 3, 2022
* Pass renderer locale to exthost, fixes #85675

* Restrict fix to macOS
rzhao271 added a commit that referenced this issue Feb 15, 2022
rzhao271 added a commit that referenced this issue Feb 15, 2022
@deepak1556 deepak1556 reopened this Feb 15, 2022
@deepak1556 deepak1556 removed the insiders-released Patch has been released in VS Code Insiders label Feb 15, 2022
@rzhao271 rzhao271 modified the milestones: February 2022, March 2022 Feb 18, 2022
@rzhao271
Copy link
Contributor

rzhao271 commented Mar 3, 2022

Here's the current situation:

  1. The original poster of this issue has already found a workaround (Set the LANG when invoking cocoapods flutter/flutter#45710)
  2. I already tried passing in Intl.DateTimeFormat().resolvedOptions().locale as the locale to the extension host, but we end up with locales such as fr that do not correspond to anything available on the OS, causing a regression in several extensions (spawn comes with a broken locale and $LANG #142857).
  3. Non-web extensions can instead use https://www.npmjs.com/package/os-locale to detect the OS locale. If we use this module ourselves, then the extension host becomes slower to spawn in general, and in the worst case, we might break some extensions again.

Therefore, I'll mark this issue as a wontfix for now.

@rzhao271 rzhao271 closed this as completed Mar 3, 2022
@rzhao271 rzhao271 removed this from the March 2022 milestone Mar 3, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 17, 2022
@bpasero bpasero moved this to ✔️ Done / Deferred in VSCode Electron Integration Jul 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug confirmed Issue has been confirmed by VS Code Team member electron Issues and items related to Electron macos Issues with VS Code on MAC/OS X upstream Issue identified as 'upstream' component related (exists outside of VS Code) wont-fix
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants
@deepak1556 @DanTup @alexdima @rzhao271 and others