Skip to content
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

[Blazor] Accept the blazor.web.js startup options format in blazor.{server|webassembly}.js #51611

Open
MackinnonBuck opened this issue Oct 24, 2023 · 8 comments · May be fixed by #56758
Open
Assignees
Labels
area-blazor Includes: Blazor, Razor Components Complexity: Simple Handling this issue should be relatively simple to tackle enhancement This issue represents an ask for new feature or an enhancement to an existing one good first issue Good for newcomers. help wanted Up for grabs. We would accept a PR to help resolve this issue Pillar: Complete Blazor Web Priority:2 Work that is important, but not critical for the release triaged

Comments

@MackinnonBuck
Copy link
Member

MackinnonBuck commented Oct 24, 2023

Problem

When a Blazor app uses blazor.web.js, the options object passed to Blazor.start() has the following format:

Blazor.start({
  ssr: { /* ... */ },
  server: { /* ... */ },
  webAssembly: { /* ... */ },
});

However, when using blazor.server.js or blazor.webassembly.js, runtime-specific options are specified as top-level properties on the options object. So, for example, if you're using blazor.webassembly.js and you attempt to customize the loading of boot resources like this:

Blazor.start({
  webAssembly: {
    loadBootResource: function (/* ... */) {
      // ...
    },
  },
});

...then it will just silently "not work". This could create a pit of failure for customers who see examples in docs using the blazor.web.js options format but are writing a Blazor WebAssembly standalone app, for example.

Solution

We should update blazor.webassembly.js and blazor.server.js to accept the options format used by blazor.web.js.

Summary Comment: #51611 (comment)

@MackinnonBuck MackinnonBuck added enhancement This issue represents an ask for new feature or an enhancement to an existing one area-blazor Includes: Blazor, Razor Components labels Oct 24, 2023
@mkArtakMSFT mkArtakMSFT added this to the .NET 9 Planning milestone Oct 25, 2023
@ghost
Copy link

ghost commented Oct 25, 2023

Thanks for contacting us.

We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@ghost
Copy link

ghost commented Dec 20, 2023

Thanks for contacting us.

We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@mkArtakMSFT mkArtakMSFT added the help candidate Indicates that the issues may be a good fit for community to help with. Requires work from eng. team label Dec 20, 2023
@mkArtakMSFT
Copy link
Member

@MackinnonBuck can you please follow the https://aka.ms/aspnet/processes/help-wanted process to turn this into a help wanted issue? Thanks!

@MackinnonBuck MackinnonBuck added Complexity: Simple Handling this issue should be relatively simple to tackle good first issue Good for newcomers. labels Dec 21, 2023
@MackinnonBuck
Copy link
Member Author

MackinnonBuck commented Dec 21, 2023

Help Wanted

Issue Summary

See #51611 (comment)

Potential Design

We can add one additional property to CircuitStartOptions and WebAssemblyStartOptions each:

export interface CircuitStartOptions {
  // <existing members>

  // Exists for compatibility with WebStartOptions.
  circuit?: CircuitStartOptions;
}

export interface WebAssemblyStartOptions {
  // <existing members>

  // Exists for compatibility with WebStartOptions.
  webAssembly?: WebAssemblyStartOptions;
}

Then we'll update Boot.Server.ts and Boot.WebAssembly.ts to "normalize" the options. Without loss of generality between circuit and WebAssembly options, something like this should work:

async function boot(options?: Partial<WebAssemblyStartOptions>): Promise<void> {
  // ...
  const normalizedOptions = options?.webAssembly ?? options ?? {};
  setWebAssemblyOptions(Promise.resolve(normalizedOptions));
}

This approach always prioritizes the "nested" options over top-level options. I don't anticipate there being any cases where nested and top-level options are specified on the same object, so the simplicity of this approach seems preferable over introducing more complicated option merging semantics.

Code References

CircuitStartOptions.ts
WebAssemblyStartOptions.ts
Boot.Server.ts
Boot.WebAssembly.ts

Additional Notes

  • If the change you are going to make will include a public API change in the framework, then the change will need to go through our API Review Process. Please read this process to take any necessary steps, to avoid any potential delays down the road with accepting this change.
  • If this is your first contribution to the repo, you may find it useful to learn How to build the repo
  • Please note, that the changes will be considered for the vNext version of the framework by default. That is, after the change gets merged in main, it will be available only in the next preview release of the framework and will be shipped in the upcoming major release of the product.

@MackinnonBuck MackinnonBuck removed their assignment Dec 21, 2023
@MackinnonBuck MackinnonBuck added help wanted Up for grabs. We would accept a PR to help resolve this issue and removed help candidate Indicates that the issues may be a good fit for community to help with. Requires work from eng. team labels Dec 21, 2023
@ghost
Copy link

ghost commented Dec 21, 2023

Looks like this issue has been identified as a candidate for community contribution. If you're considering sending a PR for this issue, look for the Summary Comment link in the issue description. That comment has been left by an engineer on our team to help you get started with handling this issue. You can learn more about our Help Wanted process here

@mkArtakMSFT mkArtakMSFT added the Priority:2 Work that is important, but not critical for the release label Jan 16, 2024
@woefulpines
Copy link

Hello, I'm starting to get my feet wet in open source development, mind if I try my hand at this issue?

@vladimir-shirmanov
Copy link

Hi there,
@MackinnonBuck your link on How to build the repo is broken.
Can you please provide a valid one?

@vladimir-shirmanov
Copy link

Hello,

I created a PR that I believe will resolve this issue, can somebody please take a look if it is valid?

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 Complexity: Simple Handling this issue should be relatively simple to tackle enhancement This issue represents an ask for new feature or an enhancement to an existing one good first issue Good for newcomers. help wanted Up for grabs. We would accept a PR to help resolve this issue Pillar: Complete Blazor Web Priority:2 Work that is important, but not critical for the release triaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants