Add support for loading satellite assemblies #20033
Conversation
ghost
added
the
pranavkm
force-pushed
the
prkrishn/loc
branch
3 times, most recently
from
1a69502 to
1b43885
Compare
| @@ -5,8 +5,7 @@ | |||
| <AdditionalMonoLinkerOptions>--disable-opt unreachablebodies --verbose --strip-security true --exclude-feature com -v false -c link -u link -b true</AdditionalMonoLinkerOptions> | |||
|
|
|||
| <_BlazorJsPath Condition="'$(_BlazorJsPath)' == ''">$(MSBuildThisFileDirectory)..\tools\blazor\blazor.webassembly.js</_BlazorJsPath> | |||
| <_BaseBlazorDistPath>dist\</_BaseBlazorDistPath> | |||
There was a problem hiding this comment.
I had to account for these in path manipulation. Removed since this isn't used any more.
| @@ -70,73 +70,68 @@ | |||
| satellite assemblies, this should include all assemblies needed to run the application. | |||
| --> | |||
| <ItemGroup> | |||
| <_BlazorJSFile Include="$(_BlazorJSPath)" /> | |||
There was a problem hiding this comment.
Slight reshuffling to make it easier to view the binary logs
| ReferenceCopyLocalPaths includes all files that are part of the build out with CopyLocalLockFileAssemblies on. | ||
| Remove assemblies that are inputs to calculating the assembly closure. Instead use the resolved outputs, since it is the minimal set. | ||
| --> | ||
| <_BlazorCopyLocalPaths Include="@(ReferenceCopyLocalPaths)" Condition="'%(Extension)' == '.dll'" /> |
There was a problem hiding this comment.
Some of this code was duplicated in the file, possibly a bad merge.
Co-Authored-By: Steve Sanderson <[email protected]>
| const resourcePromises = Promise.all(culturesToLoad | ||
| .filter(culture => satelliteResources.hasOwnProperty(culture)) | ||
| .map(culture => resourceLoader.loadResources(satelliteResources[culture], fileName => `_framework/_bin/${fileName}`)) | ||
| .flat() |
There was a problem hiding this comment.
I tried your suggestion and it looks like we need to target es2019 to use it. Are there downsides to doing that?
There was a problem hiding this comment.
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!
| @@ -16,7 +16,6 @@ public class SatelliteResourcesLoaderTest | |||
| { | |||
| [Theory] | |||
| [InlineData("fr-FR", new[] { "fr-FR", "fr" })] | |||
| [InlineData("zh-CN", new[] { "zh-CN", "zh-Hans", "zh" })] | |||
There was a problem hiding this comment.
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" })?
There was a problem hiding this comment.
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.
Fixes #17016
How this works:
navigator.languagesmn-MNismn-Cyrl, so it's not as trivial as trimming values. .NET already has the ability to do this.EntryPointInvoker. .NET calculates the closure, sends it off to JS which knows available cultures from blazor.boot.json.mono_wasm_load_assembly- it has no effect. However, we are able to load assemblies usingAssembly.Load(byte[]). JS sends bytes to .NET which it then loads.Program.MainAsyncexecute. This affords the developer to change the culture based on some ambient state such as localStorage or query.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
MainAsyncjust 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
IStringLocalizeras part ofMainAsyncand changes the culture. e.g.The default implementation caches lookups, so newly loaded assemblies do not show up. Things work fine if you use
IStringLocalizerin an ordinary way.