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

Set noDebug flag on launch url for Blazor WASM apps #2393

Merged
merged 1 commit into from
Aug 17, 2020

Conversation

captainsafia
Copy link
Member

We're adding a query parameter that will be set/unset if the Blazor WASM application is under debug.

This flag will determine whether or not the Mono runtime optimizes execution for debugging or performance.

The flag defaults to true for backwards-compatibility but we'll need to set it to false to disable debugging overhead and improve app performance. This change only affects apps built in Development mode.

This PR should be merged after dotnet/aspnetcore#24972.

Addresses dotnet/aspnetcore#24623

@captainsafia captainsafia changed the title Set noDebug flag on launch url Set noDebug flag on launch url for Blazor WASM apps Aug 17, 2020
Copy link
Contributor

@ryanbrandenburg ryanbrandenburg left a comment

Choose a reason for hiding this comment

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

Just one question that either needs no work or a small change. :shipit: once that's resolved.

const browser = {
name: JS_DEBUG_NAME,
type: configuration.browser === 'edge' ? 'pwa-msedge' : 'pwa-chrome',
request: 'launch',
timeout: configuration.timeout || 30000,
url: configuration.url || 'https://localhost:5001',
url: setNoDebugFlag(configuration.url || 'https://localhost:5001'),
Copy link
Contributor

Choose a reason for hiding this comment

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

If we don't enforce configuration.url not to have query-strings already this will break, right? If we enforce that already great, if not you might have to use a URL class/function to add the querystring in a more robust way.

Copy link
Member Author

Choose a reason for hiding this comment

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

This change should be backwards compatible. If the query param is set but the framework doesn't contain the matching updates, then the flag will no-op.

If it is not set and the framework is updated then it defaults to true.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I meant that if configuration.url = 'https://localhost:5001?somequeryString=stuff' then this new function does url:${configuration.url}?_blazor_debug=false`` then we end up with https://localhost:5001?somequeryString=stuff?_blazor_debug=false, which would result in some weird querystrings.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah I see. We decided to keep the logic simple here. We don't anticipate that a lot of people will be launching with custom query strings. If they are, it's fairly easy to work around (e.g., edit the URL)/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants