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.
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
[browser] Generate boot config as javascript module #112947
[browser] Generate boot config as javascript module #112947
Changes from all commits
55f59bb
35c4923
52115a3
d76ef82
25534c9
7a8e9dc
bab63ee
6c8540a
efdbb2a
96b9807
098e33f
273ebde
43b0d9a
4421059
361ba8d
1bf96d3
4f53191
9281240
1d078b0
695c95a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Is this because we are letting people control where
blazor.boot.json
was being loaded from?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.
Exactly
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 is there a need for us to support that moving forward?
What scenario do we think still mandates we keep this around.
I'm saying this because the more we can simplify this process the better.
Caching and all those things we are handling through the importmap work we did, and I can't really see a reason for this being loaded from elsewhere
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 think this could be in use by non-blazor customers, but I don't have anything specific
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.
What was the original motivation? Modifying the URLs to point to some CDN or subpath on the server is still valid.
This callback processes all framework assets, not just JS modules
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.
Yes, the original motivation was for letting people control the URLs of the deployed app, when we had issues with the anti-virus.
I think it's still fine for things that are "data" like the webcil, but is there a reason why we couldn't rely on relative paths for the module bits? After all, you can already change these urls during the build process modifying the relative paths of the respective assets.
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 don't have a strong opinion. Since blazor users can configure .NET runtime through a callback on
Blazor.start
, they have all the flexibility to define the assets and their URLs in a defferent way than through the loadResource callback, on the other it's a breaking change.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.
Is there a reason why you aren't using S.T.J?
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 dunno. This code was probably inspired by the similar on SDK tests. I'll try to use S.T.J in follow up