Skip to content

Conversation

@pranavkm
Copy link
Contributor

@pranavkm pranavkm commented Mar 20, 2020

Fixes #17016


How this works:

  • We want to eagerly fetch satellite assemblies for all cultures.
  • Emscripten sets up the app's culture based on the user's language preference navigator.languages
  • To calculate the closure of assemblies to load, we have to walk the graph of parent cultures. This some non-trivial jumps (e.g. parent of mn-MN is mn-Cyrl, so it's not as trivial as trimming values. .NET already has the ability to do this.
  • To this effect, we defer loading satellite assemblies until .NET code begins to execute - in this case EntryPointInvoker. .NET calculates the closure, sends it off to JS which knows available cultures from blazor.boot.json.
  • Once the app loads, we can no longer use mono_wasm_load_assembly - it has no effect. However, we are able to load assemblies using Assembly.Load(byte[]). JS sends bytes to .NET which it then loads.
  • We let Program.MainAsync execute. This affords the developer to change the culture based on some ambient state such as localStorage or query.
  • If the culture is different, as part of WebAssemblyHost.RunAsync, Blazor fetches a new set of satellite assemblies.

Doing this in two part is nice in that in the default case, the application has access to localized resources as part of MainAsync just as with any .NET application. It does mean some users may have to download an additional set of assemblies.

One edge case that falls part is if the application uses IStringLocalizer as part of MainAsync and changes the culture. e.g.

CultureInfo.DefaultThreadCurrentCulture = GetCultureFromPath(..);
ApplicationMessage =  Host.Services.GetRequiredService<IStringLocalizer<Messages>>()["AppMessage"];

The default implementation caches lookups, so newly loaded assemblies do not show up. Things work fine if you use IStringLocalizer in an ordinary way.

@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Mar 20, 2020
@pranavkm pranavkm force-pushed the prkrishn/loc branch 3 times, most recently from 1a69502 to 1b43885 Compare March 23, 2020 22:14
@pranavkm pranavkm marked this pull request as ready for review March 23, 2020 22:14
@pranavkm pranavkm added this to the blazor-wasm-3.2-preview4 milestone Mar 23, 2020
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to account for these in path manipulation. Removed since this isn't used any more.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Slight reshuffling to make it easier to view the binary logs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some of this code was duplicated in the file, possibly a bad merge.

Co-Authored-By: Steve Sanderson <SteveSandersonMS@users.noreply.github.com>
const resourcePromises = Promise.all(culturesToLoad
.filter(culture => satelliteResources.hasOwnProperty(culture))
.map(culture => resourceLoader.loadResources(satelliteResources[culture], fileName => `_framework/_bin/${fileName}`))
.flat()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried your suggestion and it looks like we need to target es2019 to use it. Are there downsides to doing that?

Copy link
Member

Choose a reason for hiding this comment

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

Browser support for .flat looks reasonable, however it's not like we're saving ourselves dozens of lines of code here.

Your original implementation looks totally fine to me, too!

@ghost ghost closed this Mar 25, 2020
@pranavkm pranavkm reopened this Mar 25, 2020
@pranavkm pranavkm changed the base branch from blazor-wasm-preview4 to blazor-wasm March 25, 2020 17:12
{
[Theory]
[InlineData("fr-FR", new[] { "fr-FR", "fr" })]
[InlineData("zh-CN", new[] { "zh-CN", "zh-Hans", "zh" })]
Copy link
Member

Choose a reason for hiding this comment

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

Is the issue that Windows has a different opinion than Mac/Linux for this culture?

If so it seems reasonable just not to have this particular test case. Is there any other test case with nontrivial rules (not of the form { "X-Y-Z", "X-Y", "X" })?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, doesn't look like there are any non-trivial cases in Linux (haven't tested OSX). I wonder if the Linux implementation just uses the trivial fallback behavior.

@SteveSandersonMS SteveSandersonMS self-requested a review March 26, 2020 13:41
@pranavkm pranavkm merged commit 82d05ae into blazor-wasm Mar 26, 2020
@pranavkm pranavkm deleted the prkrishn/loc branch March 26, 2020 20:58
@pranavkm pranavkm restored the prkrishn/loc branch May 28, 2020 18:09
@pranavkm pranavkm deleted the prkrishn/loc branch May 28, 2020 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-blazor Includes: Blazor, Razor Components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants