-
Notifications
You must be signed in to change notification settings - Fork 10.7k
Fix: Apply Blazor WebAssembly JS Initializer configurations (loadBootResource, environment) before runtime initialization #65696
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.
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
Open
muharremtekin
wants to merge
11
commits into
dotnet:main
Choose a base branch
from
muharremtekin:fix/issue-54358-js-initializers-timing
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
7d859f3
feat: Implement MonoPlatform in TypeScript with Jest testing and add …
muharremtekin a2e8375
Merge branch 'main' into fix/issue-54358-js-initializers-timing
muharremtekin f4e5e74
Fix TypeScript compilation error in InitializerTiming test
muharremtekin fe6f0f2
Merge branch 'fix/issue-54358-js-initializers-timing' of https://gith…
muharremtekin 81482e0
test: Add E2E tests for Blazor Web JS initializers covering modern an…
muharremtekin 9938609
fix: resolve TS2345 type error in MonoPlatform.InitializerTiming.test.ts
muharremtekin 99e9a24
Merge branch 'main' into fix/issue-54358-js-initializers-timing
muharremtekin 7b9aba2
Merge branch 'main' into fix/issue-54358-js-initializers-timing
muharremtekin f651e6c
Merge branch 'main' into fix/issue-54358-js-initializers-timing
muharremtekin 5bf52b4
Merge branch 'main' into fix/issue-54358-js-initializers-timing
muharremtekin 1ca08e5
Merge branch 'main' into fix/issue-54358-js-initializers-timing
muharremtekin File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
196 changes: 196 additions & 0 deletions
196
src/Components/Web.JS/test/MonoPlatform.InitializerTiming.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,196 @@ | ||
| /** | ||
| * Test: JS Initializers beforeStart timing fix (Issue #54358) | ||
| * | ||
| * These tests validate the prepareRuntimeConfig function from MonoPlatform.ts | ||
| * to ensure that dotnet.with*() calls happen AFTER onConfigLoaded callback invokes | ||
| * fetchAndInvokeInitializers (which runs beforeStart JS initializer callbacks). | ||
| * | ||
| * Evidence from dotnet/runtime source (src/mono/browser/runtime/loader/config.ts): | ||
| * - onConfigLoaded is explicitly `await`ed by the runtime | ||
| * - Comment in source: "scripts need to be loaded before onConfigLoaded because | ||
| * Blazor calls `beforeStart` export in onConfigLoaded" | ||
| * - After onConfigLoaded returns, normalizeConfig() runs again | ||
| * - Resource downloads begin AFTER this point | ||
| * - Type signature: onConfigLoaded?: (config: MonoConfig) => void | Promise<void> | ||
| */ | ||
| import { describe, test, expect, jest, beforeEach } from '@jest/globals'; | ||
|
|
||
| // ---------- Mocks ---------- | ||
| // jest.mock() calls are HOISTED before const declarations, so we must NOT reference | ||
| // any const from outer scope inside the mock factory. Instead, we get a reference | ||
| // to the mock function via require() after mocking. | ||
|
|
||
| jest.mock('../src/JSInitializers/JSInitializers.WebAssembly', () => ({ | ||
| fetchAndInvokeInitializers: jest.fn(), | ||
| })); | ||
|
|
||
| jest.mock('../src/GlobalExports', () => ({ | ||
| Blazor: { _internal: {} }, | ||
| })); | ||
|
|
||
| jest.mock('../src/BootErrors', () => ({ | ||
| showErrorNotification: jest.fn(), | ||
| })); | ||
|
|
||
| jest.mock('../src/Platform/Mono/MonoDebugger', () => ({ | ||
| attachDebuggerHotkey: jest.fn(), | ||
| })); | ||
|
|
||
| // Import modules after mocks | ||
| import { prepareRuntimeConfig } from '../src/Platform/Mono/MonoPlatform'; | ||
| import { fetchAndInvokeInitializers } from '../src/JSInitializers/JSInitializers.WebAssembly'; | ||
|
|
||
| // Cast to jest mock for type safety | ||
| const mockFetchAndInvokeInitializers = fetchAndInvokeInitializers as any; | ||
|
|
||
| // ---------- Helpers ---------- | ||
|
|
||
| function createMockDotnetBuilder() { | ||
| const callOrder: string[] = []; | ||
| return { | ||
| callOrder, | ||
| builder: { | ||
| withApplicationCulture: jest.fn(() => { callOrder.push('withApplicationCulture'); }), | ||
| withApplicationEnvironment: jest.fn(() => { callOrder.push('withApplicationEnvironment'); }), | ||
| withResourceLoader: jest.fn(() => { callOrder.push('withResourceLoader'); }), | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| function createMockMonoConfig(overrides: Record<string, any> = {}) { | ||
| return { | ||
| environmentVariables: {}, | ||
| applicationEnvironment: 'Production', | ||
| applicationCulture: 'en-US', | ||
| resources: {}, | ||
| ...overrides, | ||
| }; | ||
| } | ||
|
|
||
| // ---------- Tests ---------- | ||
|
|
||
| describe('prepareRuntimeConfig — onConfigLoaded timing (Issue #54358)', () => { | ||
| beforeEach(() => { | ||
| mockFetchAndInvokeInitializers.mockReset(); | ||
| }); | ||
|
|
||
| test('dotnet.withResourceLoader is called AFTER fetchAndInvokeInitializers (beforeStart)', async () => { | ||
| const callOrder: string[] = []; | ||
| const customLoader = jest.fn(); | ||
| const options: Record<string, any> = {}; | ||
|
|
||
| const { builder } = createMockDotnetBuilder(); | ||
| builder.withResourceLoader = jest.fn((loader: any) => { | ||
| callOrder.push('withResourceLoader'); | ||
| expect(loader).toBe(customLoader); | ||
| }) as any; | ||
|
|
||
| // Simulate beforeStart setting loadBootResource on the options object | ||
| mockFetchAndInvokeInitializers.mockImplementation(async (opts: any) => { | ||
| callOrder.push('fetchAndInvokeInitializers'); | ||
| opts.loadBootResource = customLoader; | ||
| opts.environment = 'Development123'; | ||
| return {} as any; | ||
| }); | ||
|
|
||
| const moduleConfig = prepareRuntimeConfig(options as any, builder as any); | ||
| await moduleConfig.onConfigLoaded!(createMockMonoConfig() as any); | ||
|
|
||
| // Core invariant: beforeStart runs BEFORE dotnet.with*() | ||
| expect(callOrder.indexOf('fetchAndInvokeInitializers')) | ||
| .toBeLessThan(callOrder.indexOf('withResourceLoader')); | ||
| expect(builder.withResourceLoader).toHaveBeenCalledWith(customLoader); | ||
| }); | ||
|
|
||
| test('dotnet.withApplicationEnvironment is called AFTER beforeStart sets environment', async () => { | ||
| const options: Record<string, any> = {}; | ||
| const { builder, callOrder } = createMockDotnetBuilder(); | ||
|
|
||
| mockFetchAndInvokeInitializers.mockImplementation(async (opts: any) => { | ||
| callOrder.push('fetchAndInvokeInitializers'); | ||
| opts.environment = 'Staging'; | ||
| return {} as any; | ||
| }); | ||
|
|
||
| const moduleConfig = prepareRuntimeConfig(options as any, builder as any); | ||
| await moduleConfig.onConfigLoaded!(createMockMonoConfig() as any); | ||
|
|
||
| expect(callOrder.indexOf('fetchAndInvokeInitializers')) | ||
| .toBeLessThan(callOrder.indexOf('withApplicationEnvironment')); | ||
| expect(builder.withApplicationEnvironment).toHaveBeenCalledWith('Staging'); | ||
| }); | ||
|
|
||
| test('dotnet.with*() is NOT called when beforeStart does not set those options', async () => { | ||
| const options: Record<string, any> = {}; | ||
| const { builder } = createMockDotnetBuilder(); | ||
|
|
||
| mockFetchAndInvokeInitializers.mockResolvedValue({} as any); | ||
|
|
||
| const moduleConfig = prepareRuntimeConfig(options as any, builder as any); | ||
| await moduleConfig.onConfigLoaded!(createMockMonoConfig() as any); | ||
|
|
||
| expect(builder.withResourceLoader).not.toHaveBeenCalled(); | ||
| expect(builder.withApplicationEnvironment).not.toHaveBeenCalled(); | ||
| expect(builder.withApplicationCulture).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| test('configureRuntime is called AFTER fetchAndInvokeInitializers', async () => { | ||
| const callOrder: string[] = []; | ||
| const customConfigureRuntime = jest.fn(() => { callOrder.push('configureRuntime'); }); | ||
| const options: Record<string, any> = {}; | ||
| const { builder } = createMockDotnetBuilder(); | ||
|
|
||
| mockFetchAndInvokeInitializers.mockImplementation(async (opts: any) => { | ||
| callOrder.push('fetchAndInvokeInitializers'); | ||
| opts.configureRuntime = customConfigureRuntime; | ||
| return {} as any; | ||
| }); | ||
|
|
||
| const moduleConfig = prepareRuntimeConfig(options as any, builder as any); | ||
| await moduleConfig.onConfigLoaded!(createMockMonoConfig() as any); | ||
|
|
||
| expect(callOrder.indexOf('fetchAndInvokeInitializers')) | ||
| .toBeLessThan(callOrder.indexOf('configureRuntime')); | ||
| expect(customConfigureRuntime).toHaveBeenCalledWith(builder); | ||
| }); | ||
|
|
||
| test('onConfigLoadedCallback fires before fetchAndInvokeInitializers', async () => { | ||
| const callOrder: string[] = []; | ||
| const options: Record<string, any> = {}; | ||
| const { builder } = createMockDotnetBuilder(); | ||
|
|
||
| mockFetchAndInvokeInitializers.mockImplementation(async () => { | ||
| callOrder.push('fetchAndInvokeInitializers'); | ||
| return {} as any; | ||
| }); | ||
|
|
||
| const onConfigLoadedCallback = jest.fn(() => { | ||
| callOrder.push('onConfigLoadedCallback'); | ||
| }); | ||
|
|
||
| const moduleConfig = prepareRuntimeConfig(options as any, builder as any, onConfigLoadedCallback as any); | ||
| await moduleConfig.onConfigLoaded!(createMockMonoConfig() as any); | ||
|
|
||
| expect(callOrder.indexOf('onConfigLoadedCallback')) | ||
| .toBeLessThan(callOrder.indexOf('fetchAndInvokeInitializers')); | ||
| }); | ||
|
|
||
| test('pre-configured options (via Blazor.start) are also applied in onConfigLoaded', async () => { | ||
| const customLoader = jest.fn(); | ||
| const options: Record<string, any> = { | ||
| loadBootResource: customLoader, | ||
| environment: 'Production', | ||
| applicationCulture: 'tr-TR', | ||
| }; | ||
| const { builder } = createMockDotnetBuilder(); | ||
|
|
||
| mockFetchAndInvokeInitializers.mockResolvedValue({} as any); | ||
|
|
||
| const moduleConfig = prepareRuntimeConfig(options as any, builder as any); | ||
| await moduleConfig.onConfigLoaded!(createMockMonoConfig() as any); | ||
|
|
||
| expect(builder.withResourceLoader).toHaveBeenCalledWith(customLoader); | ||
| expect(builder.withApplicationEnvironment).toHaveBeenCalledWith('Production'); | ||
| expect(builder.withApplicationCulture).toHaveBeenCalledWith('tr-TR'); | ||
| }); | ||
| }); |
7 changes: 7 additions & 0 deletions
7
src/Components/Web.JS/test/__mocks__/@microsoft/dotnet-js-interop.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| // Stub module for @microsoft/dotnet-js-interop (not a real npm package in this repo) | ||
| module.exports = { | ||
| DotNet: { | ||
| attachDispatcher: function () { return {}; }, | ||
| attachReviver: function () { }, | ||
| }, | ||
| }; |
9 changes: 9 additions & 0 deletions
9
src/Components/Web.JS/test/__mocks__/@microsoft/dotnet-runtime.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| // Stub module for @microsoft/dotnet-runtime (not a real npm package in this repo) | ||
| module.exports = { | ||
| GlobalizationMode: Object.freeze({ | ||
| Sharded: 'sharded', | ||
| All: 'all', | ||
| Invariant: 'invariant', | ||
| Custom: 'custom', | ||
| }), | ||
| }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@maraf why we are doing
fetchAndInvokeInitializershere and not only in runtime ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initializers is a blazor feature, not a wasm feature
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/dotnet/runtime/blob/07dc6fab46e5235c4d5ccc12c8a121c974832099/src/mono/browser/runtime/loader/config.ts#L260-L262
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know wasm has it too, it got added there too when we did a refactor a couple years later. The blazor feature predates the wasm feature #34798
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, why we didn't finish the refactoring and unify it ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because we can't. This is used by libraries and we shouldn't break them, and I believe there were some things that could be done in the Blazor version that couldn't be done in the wasm version.
To be very clear, we can't just remove this feature for blazor. it's used on server, ssr, webview and (wasm) where wasm also added its own hooks. And it's the pattern libraries have to bring their JS. This should keep working for backwards compatibility.