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.

Sign up for GitHub

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

Jump to bottom

[BUG] package.json and package-lock.json order is different when used Czech locale #2829

Closed
jakubjosef opened this issue Mar 5, 2021 · 4 comments
Closed

[BUG] package.json and package-lock.json order is different when used Czech locale #2829

jakubjosef opened this issue Mar 5, 2021 · 4 comments
Assignees
@wraithgar
Labels
Bug thing that needs fixing Priority 2 secondary priority issue Release 7.x work is associated with a specific npm 7 release

Comments

@jakubjosef
Copy link

jakubjosef commented Mar 5, 2021 •
edited
Loading

Current Behavior:

package-lock and package.json order is different when used non English (Czech) locale

Expected Behavior:

Sorting should be locale-agnostic to not change package-lock JSON back and forth.

Steps To Reproduce:

  1. Clone repo created with English locale
  2. Run npm install
  3. I see different order for packages with ch in name (eg. chai)

Environment:

  • OS: macOS 11.2.2
  • Node: 14.16.0
  • npm: 7.6.1

It looks like similar problem as it's reported here
npm/npm#17048

I think the root cause is different order of letters. In Czech alphabet CH letter stands after H, but in English alphabet it's just C.

Example in package.json (left is Czech locale, right is English)

image

Workaround

If someone is facing the same issue in can be fixed by defining alias alias npm='LANG="en_US.UTF-8" LC_ALL="en_US.UTF-8" npm' in bash_profile/.zshrc or similar file depends to shell you are using.
@jakubjosef jakubjosef added Bug thing that needs fixing Needs Triage needs review for next steps Release 7.x work is associated with a specific npm 7 release labels Mar 5, 2021
@darcyclarke darcyclarke added Priority 2 secondary priority issue and removed Needs Triage needs review for next steps labels Apr 9, 2021
@padinko
Copy link

padinko commented May 4, 2021

same in Windows 10 with Slovak language, node v14.16.1, npm v7.11.2
i can't find workaround for windows

This was referenced May 5, 2021
Closed
Closed
@padinko
Copy link

padinko commented May 5, 2021

I make pull requests to fix this

npm/arborist#276 will fix sorting in package.json
isaacs/json-stringify-nice#5 will fix sorting in package-lock.json

so this 2 packeses will need to be updated to fix this

@wraithgar
Copy link
Member

Thank you @padinko!

@wraithgar wraithgar self-assigned this May 5, 2021
isaacs added a commit to npm/arborist that referenced this issue May 6, 2021
@isaacs
guard against locale-specific sorting
ea12f81 
Node will use the current environment's locale sorting for
String.localeCompare.  Thankfully, String.localeCompare can take a
second argument to specify the locale.  Up until Node 13, any valid
argument provided to the function _may_ be respected if Node was
compiled with Intl support, or would always be treated as `'en'`
otherwise.

The solution is to always set the locale to `'en'`.  An alternative
solution would be to compare strings based on byte order, but this does
not provide ideal ergonomis.

To verify this, the locale is set in the test environment to `'sk'`,
which has a significantly different alphabet, including sorting words
starting with `'ch'` _after_ words starting with `'d'`.

Re: npm/cli#2829
Fix: #276
@isaacs isaacs mentioned this issue May 6, 2021
Merged
isaacs added a commit to isaacs/node-glob that referenced this issue May 6, 2021
@isaacs
force 'en' locale in string sorting
66c102e 
Re: npm/cli#2829
isaacs added a commit to npm/ignore-walk that referenced this issue May 6, 2021
@isaacs
Avoid locale-specific sorting issues
ea6c74b 
Re: npm/cli#2829
isaacs pushed a commit to isaacs/json-stringify-nice that referenced this issue May 6, 2021
@padinko @isaacs
Make sort locale independent
08736fa 
Re: npm/cli#2829

PR-URL: #5
Credit: @padinko
Close: #5
Reviewed-by: @isaacs

EDIT(@isaacs): added tests
isaacs added a commit to npm/libnpmexec that referenced this issue May 6, 2021
@isaacs
Specify 'en' locale to String.localeCompare
7a0269a 
No test added, since there's nothing in this module that particularly
depends on string sorting, but it's a best practice all the same.

Re: npm/cli#2829
@isaacs isaacs mentioned this issue May 6, 2021
Merged
isaacs added a commit to npm/npm-packlist that referenced this issue May 6, 2021
@isaacs
Avoid locale-dependent sorting issues
36c7668 
Force all uses of String.localeCompare to use the 'en' locale.

Re: npm/cli#2829
isaacs added a commit that referenced this issue May 6, 2021
@isaacs
Avoid locale-specific string sorting issues
eedb6f0 
This adds the 'en' locale to all instances of String.localeCompare
within the CLI codebase.

Tests added for the cases where we're sorting arbitrary user-generated
data.  The tests rely on the fact that 'ch' sorts after 'd' in the
`'sk'` locale, but ahead of `'d'` in the `'en'` locale.  To ensure that
this is the default behavior if no locale is specified, `LC_ALL=sk` is
set in the test environment.

Other instances of `localeCompare` sort data that the cli controls, so
no tests were added.

Re: #2829
@isaacs isaacs mentioned this issue May 6, 2021
Merged
isaacs added a commit to npm/arborist that referenced this issue May 6, 2021
@isaacs
guard against locale-specific sorting
dc3590b 
Node will use the current environment's locale sorting for
String.localeCompare.  Thankfully, String.localeCompare can take a
second argument to specify the locale.  Up until Node 13, any valid
argument provided to the function _may_ be respected if Node was
compiled with Intl support, or would always be treated as `'en'`
otherwise.

The solution is to always set the locale to `'en'`.  An alternative
solution would be to compare strings based on byte order, but this does
not provide ideal ergonomis.

To verify this, the locale is set in the test environment to `'sk'`,
which has a significantly different alphabet, including sorting words
starting with `'ch'` _after_ words starting with `'d'`.

Re: npm/cli#2829
Fix: #276
isaacs added a commit to npm/arborist that referenced this issue May 7, 2021
@isaacs
guard against locale-specific sorting
bf38e1b 
Node will use the current environment's locale sorting for
String.localeCompare.  Thankfully, String.localeCompare can take a
second argument to specify the locale.  Up until Node 13, any valid
argument provided to the function _may_ be respected if Node was
compiled with Intl support, or would always be treated as `'en'`
otherwise.

The solution is to always set the locale to `'en'`.  An alternative
solution would be to compare strings based on byte order, but this does
not provide ideal ergonomis.

To verify this, the locale is set in the test environment to `'sk'`,
which has a significantly different alphabet, including sorting words
starting with `'ch'` _after_ words starting with `'d'`.

Re: npm/cli#2829
Fix: #276
isaacs added a commit that referenced this issue May 7, 2021
@isaacs
Avoid locale-specific string sorting issues
fa72535 
This adds the 'en' locale to all instances of String.localeCompare
within the CLI codebase.

Tests added for the cases where we're sorting arbitrary user-generated
data.  The tests rely on the fact that 'ch' sorts after 'd' in the
`'sk'` locale, but ahead of `'d'` in the `'en'` locale.  To ensure that
this is the default behavior if no locale is specified, `LC_ALL=sk` is
set in the test environment.

Other instances of `localeCompare` sort data that the cli controls, so
no tests were added.

Re: #2829
isaacs added a commit that referenced this issue May 7, 2021
@isaacs
Avoid locale-specific string sorting issues
1a759f9 
This adds the 'en' locale to all instances of String.localeCompare
within the CLI codebase.

Tests added for the cases where we're sorting arbitrary user-generated
data.  The tests rely on the fact that 'ch' sorts after 'd' in the
`'sk'` locale, but ahead of `'d'` in the `'en'` locale.  To ensure that
this is the default behavior if no locale is specified, `LC_ALL=sk` is
set in the test environment.

Other instances of `localeCompare` sort data that the cli controls, so
no tests were added.

Re: #2829
isaacs added a commit that referenced this issue May 10, 2021
@isaacs
Avoid locale-specific string sorting issues
937dbd6 
This adds the 'en' locale to all instances of String.localeCompare
within the CLI codebase.

Tests added for the cases where we're sorting arbitrary user-generated
data.  The tests rely on the fact that 'ch' sorts after 'd' in the
`'sk'` locale, but ahead of `'d'` in the `'en'` locale.  To ensure that
this is the default behavior if no locale is specified, `LC_ALL=sk` is
set in the test environment.

Other instances of `localeCompare` sort data that the cli controls, so
no tests were added.

Re: #2829
isaacs added a commit that referenced this issue May 10, 2021
@isaacs
Avoid locale-specific string sorting issues
a513345 
This adds the 'en' locale to all instances of String.localeCompare
within the CLI codebase.

Tests added for the cases where we're sorting arbitrary user-generated
data.  The tests rely on the fact that 'ch' sorts after 'd' in the
`'sk'` locale, but ahead of `'d'` in the `'en'` locale.  To ensure that
this is the default behavior if no locale is specified, `LC_ALL=sk` is
set in the test environment.

Other instances of `localeCompare` sort data that the cli controls, so
no tests were added.

Re: #2829
wraithgar pushed a commit to npm/libnpmexec that referenced this issue May 10, 2021
@isaacs
fix(add): Specify 'en' locale to String.localeCompare (#3)
7d185c3 
No test added, since there's nothing in this module that particularly
depends on string sorting, but it's a best practice all the same.

Re: npm/cli#2829
isaacs added a commit to npm/arborist that referenced this issue May 10, 2021
@isaacs
guard against locale-specific sorting
8fbe794 
Node will use the current environment's locale sorting for
String.localeCompare.  Thankfully, String.localeCompare can take a
second argument to specify the locale.  Up until Node 13, any valid
argument provided to the function _may_ be respected if Node was
compiled with Intl support, or would always be treated as `'en'`
otherwise.

The solution is to always set the locale to `'en'`.  An alternative
solution would be to compare strings based on byte order, but this does
not provide ideal ergonomis.

To verify this, the locale is set in the test environment to `'sk'`,
which has a significantly different alphabet, including sorting words
starting with `'ch'` _after_ words starting with `'d'`.

Re: npm/cli#2829
Fix: #276
isaacs added a commit to npm/arborist that referenced this issue May 10, 2021
@isaacs
guard against locale-specific sorting
a843eea 
Node will use the current environment's locale sorting for
String.localeCompare.  Thankfully, String.localeCompare can take a
second argument to specify the locale.  Up until Node 13, any valid
argument provided to the function _may_ be respected if Node was
compiled with Intl support, or would always be treated as `'en'`
otherwise.

The solution is to always set the locale to `'en'`.  An alternative
solution would be to compare strings based on byte order, but this does
not provide ideal ergonomis.

To verify this, the locale is set in the test environment to `'sk'`,
which has a significantly different alphabet, including sorting words
starting with `'ch'` _after_ words starting with `'d'`.

Re: npm/cli#2829
Fix: #276
isaacs added a commit that referenced this issue May 10, 2021
@isaacs
Avoid locale-specific string sorting issues
530427c 
This adds the 'en' locale to all instances of String.localeCompare
within the CLI codebase.

Tests added for the cases where we're sorting arbitrary user-generated
data.  The tests rely on the fact that 'ch' sorts after 'd' in the
`'sk'` locale, but ahead of `'d'` in the `'en'` locale.  To ensure that
this is the default behavior if no locale is specified, `LC_ALL=sk` is
set in the test environment.

Other instances of `localeCompare` sort data that the cli controls, so
no tests were added.

Re: #2829
wraithgar pushed a commit to npm/arborist that referenced this issue May 10, 2021
@isaacs @wraithgar
guard against locale-specific sorting
571e9e4 
Node will use the current environment's locale sorting for
String.localeCompare.  Thankfully, String.localeCompare can take a
second argument to specify the locale.  Up until Node 13, any valid
argument provided to the function _may_ be respected if Node was
compiled with Intl support, or would always be treated as `'en'`
otherwise.

The solution is to always set the locale to `'en'`.  An alternative
solution would be to compare strings based on byte order, but this does
not provide ideal ergonomis.

To verify this, the locale is set in the test environment to `'sk'`,
which has a significantly different alphabet, including sorting words
starting with `'ch'` _after_ words starting with `'d'`.

Re: npm/cli#2829
Fix: #276
wraithgar pushed a commit that referenced this issue May 10, 2021
@isaacs @wraithgar
fix(packages): locale-agnostic string sorting
1d09214 
This adds the 'en' locale to all instances of String.localeCompare
within the CLI codebase.

Tests added for the cases where we're sorting arbitrary user-generated
data.  The tests rely on the fact that 'ch' sorts after 'd' in the
`'sk'` locale, but ahead of `'d'` in the `'en'` locale.  To ensure that
this is the default behavior if no locale is specified, `LC_ALL=sk` is
set in the test environment.

Other instances of `localeCompare` sort data that the cli controls, so
no tests were added.

Re: #2829

PR-URL: #3203
Credit: @isaacs
Close: #3203
Reviewed-by: @ruyadorno
@jakubjosef
Copy link
Author

I can confirm this is now fixed in NPM. Thank you, closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@wraithgar wraithgar

Labels
Bug thing that needs fixing Priority 2 secondary priority issue Release 7.x work is associated with a specific npm 7 release
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@wraithgar @darcyclarke @jakubjosef @padinko