Skip to content

fix: compatibility for modern icu#9887

Open
nwidynski wants to merge 1 commit intoadobe:mainfrom
nwidynski:fix-icu-78
Open

fix: compatibility for modern icu#9887
nwidynski wants to merge 1 commit intoadobe:mainfrom
nwidynski:fix-icu-78

Conversation

@nwidynski
Copy link
Copy Markdown
Contributor

Closes failures when running test suite locally.

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

value = replaceAll(value, "'", this.symbols.group);
}

// On newer ICU versions, the special single quote has been normalized, so we need to backport.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure I understand why this change is needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

See #9887 (comment) - apparently de-CH doesn't always resolve to the special group character, thereby causing the test for it to fail as NaN is parsed.

@snowystinger
Copy link
Copy Markdown
Member

Thanks for the PR.

You're running on a newer version than v24.14.1?

Instead of having the tests pass for both, we should either lock the nvm to the version which works, or update the node version on CI and everywhere else along with the tests.

We're making the tests harder to update in the future should it change in some way not already covered. This is how we've handled it in the past as well.

@nwidynski
Copy link
Copy Markdown
Contributor Author

nwidynski commented Apr 8, 2026

@snowystinger I'm running on 24.14.0, OSX 26.2, with US and English, albeit in Europe/Berlin TZ. Curiously enough, this results in the following. Don't mind the mx part (its just an alias to path activate the toolchain).

~/dev/public/rsp main* ❯ mx node -e "console.log(new Intl.NumberFormat('de-CH').formatToParts(1000).find(p => p.type === 'group').value)"                                                                                                                                         10:40:17 AM
' // U+0027
~/dev/public/rsp main* ❯ mx bun -e "console.log(new Intl.NumberFormat('de-CH').formatToParts(1000).find(p => p.type === 'group').value)"                                                                                                                                          10:40:28 AM
’ // U+2019
~/dev/public/rsp main* ❯ mx node -e "console.log(new Intl.DateTimeFormat('en-US-u-ca-ethiopic', {year: 'numeric', era: 'narrow', timeZone: 'UTC'}).formatToParts(0).find(p => p.type === 'era').value)"                                                                           11:16:52 AM
AM
~/dev/public/rsp main* ❯ mx bun -e "console.log(new Intl.DateTimeFormat('en-US-u-ca-ethiopic', {year: 'numeric', era: 'narrow', timeZone: 'UTC'}).formatToParts(0).find(p => p.type === 'era').value)"                                                                            11:16:57 AM
ERA1
~/dev/public/rsp main* ❯ mx node -v                                                                                                                                                                                                                                               10:40:40 AM
v24.14.0
~/dev/public/rsp main* ❯ mx bun -v                                                                                                                                                                                                                                                10:41:40 AM
1.3.11

Afaik, CircleCI appears to be locked to 24.13. I get the same issue on my system when I run with v24.13.1 though, so I was wondering why CI is passing. Since I don't use nvm, I may be running another binary for the same version??

Edit: My colleague, who's using nvm got the same issue on v24.13. Curious to learn why CI is passing then 🤷

@snowystinger
Copy link
Copy Markdown
Member

snowystinger commented Apr 10, 2026

UPDATE i mispoke about 24.14.1, the comment has been updated.

Test with different node versions
24.13.0 passes -> circleci
24.13.1 fails
24.14.0 fails
24.14.1 fails

Best guess is that ICU fixed something. I'm ok leaving the backtick vs apostrophe, that seems useful like some of our other conversions for uncommon characters.

I think we can just update any test to use the new correct value.

I don't think we need to do the era check in the test, we can just change the nvmrc version for the repo to ^24.14 I think it can take a semver range and update the circleci config, will that still work with your tool?

@nwidynski
Copy link
Copy Markdown
Contributor Author

nwidynski commented Apr 10, 2026

I'm using mise-en-place (alias mise x -- -> mx). Unfortunately it only supports exact versions for idiomatic install. I've previously worked around this when we had 20.x , but it is really dreadful to maintain custom tool version aliases for various mutations of version ranges 😓.

I would appreciate either locking to a specific version or otherwise supporting both scenarios in tests, but I will be fine if you don't want to go that route - just need to configure caret aliases once until upstream supports it.

Edit: Nvmrc does not support semver ranges, see nvm-sh/nvmrc#2

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.

2 participants