search

scratchfoundation / scratch-www

Standalone web client for Scratch
https://scratch.mit.edu
BSD 3-Clause "New" or "Revised" License
1.58k stars 834 forks source link

Update react-intl to latest version #1452

Open mewtaylor opened 7 years ago

mewtaylor commented 7 years ago

Process:

Here are the upgrade guides: https://formatjs.io/docs/react-intl/upgrade-guide-3x https://formatjs.io/docs/react-intl/upgrade-guide-4x https://formatjs.io/docs/react-intl/upgrade-guide-5x

rschamp commented 3 years ago

When we upgraded react-intl, we hit an error due to scratch-l10n trying to import locales from react-intl that no longer exist: https://travis-ci.com/github/LLK/scratch-www/builds/219346737, this seems to be due to the import on build-locales. This step is where scratch-l10n supplies a list of locales along with locale data for each. Currently it imports a lot of locale data from react-intl, but now react-intl removed that data in preference for using a polyfill. This was all used by scratch-www in order to add locale data, but this method no longer exists... in preference for using a polyfill.

We're not sure yet if react-intl needs to be in sync across scratch-www, scratch-gui, scratch-l10n, (others?...)

apple502j commented 3 years ago

One important thing is that v3 removed IntlShape. That'd require rewriting every code we know...

rschamp commented 3 years ago

With the work we've done on scratch-l10n, we can upgrade scratch-www to v3 without breaking GUI. While v3 removed intlShape it provides IntlShape as an interface, which we can supposedly use instead.

Next up is polyfilling, we decided we probably want to remove the Intl and any Intl-related polyfills from the polyfill bundle we currently have, and instead use polyfill-library in a new entry, so we can selectively polyfill features, as well as provide locale data as necessary in the same file. (Eventually we could do away with the polyfill bundle and use this entry point for the other polyfills we're using too. But to begin with we'll just use it for Intl-related stuff.)

apple502j commented 3 years ago

Ah, it all makes sense now - the code copy-pasted from TypeScript handbook, and now we use IntlShape interface...

No, you can't just sed s/intlShape/IntlShape/g. While intlShape is for PropTypes, IntlShape is a TypeScript interface. Which probably means: we need to migrate to TypeScript (or define intlShape ourselves/just use PropTypes.object ignoring linters)

Another thing: we might want to consider if polyfills are actually needed. We currently polyfill everything from Array.prototype.forEach (es5-shim) to things like matchMedia, btoa, etc.

chrisgarrity commented 3 years ago

Confirmed that a polyfill like @formatjs/intl-realtivetimeformat does not check for or include polyfills for intl components it requires, such as @formatjs/intl-pluralrules.

chrisgarrity commented 3 years ago

https://github.com/LLK/scratch-www/tree/dynamic-intl-polyfill is the start of switching to polyfills. init.js is correctly determining whether to polyfill or not. Right now, it's still using react-intl 2.x, so localedata is still being loaded in intl.jsx.

Next steps:

rschamp commented 2 years ago

In https://github.com/LLK/scratch-www/pull/6243 we began polyfilling dynamically. Next we should generate and supply this dynamic polyfill behavior from scratch-l10n so that it tracks with updates to the language list. We also can now move on to upgrading to react-intl 3.x!

rschamp commented 2 years ago

We hit a hitch with the polyfill and hopefully resolved it in #6285

rschamp commented 2 years ago

With #6285 merged, we rebased and continued work on https://github.com/LLK/scratch-www/tree/update-react-intl-3. We removed intl.jsx, and updated uses of intlShape to intl.IntlShape (now a typescript definition). It builds, but tests are failing apparently due to the way we're using createIntl: https://app.circleci.com/pipelines/github/LLK/scratch-www/7312/workflows/81a264cf-3ffe-424d-9a4c-5b756ac5d6f4/jobs/9373