-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
feat: support disabling domain sharding with a query param #17177
feat: support disabling domain sharding with a query param #17177
Conversation
Codecov Report
@@ Coverage Diff @@
## master #17177 +/- ##
==========================================
+ Coverage 76.83% 76.93% +0.10%
==========================================
Files 1039 1039
Lines 55561 55583 +22
Branches 7570 7577 +7
==========================================
+ Hits 42690 42764 +74
+ Misses 12621 12569 -52
Partials 250 250
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
/testenv up |
@etr2460 Ephemeral environment spinning up at http://34.219.21.46:8080. Credentials are |
This is tested in our internal environment and we can verify that domain sharding is disabled when the query param is set |
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.
LGTM with one minor suggestion.
// don't do domain sharding if a certain query param is set | ||
const disableDomainSharding = | ||
new URLSearchParams(window.location.search).get('disableDomainSharding') === | ||
'1'; |
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.
Why not just return even earlier so you don't even have to parse the bootstrapData
?
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.
done
2e7067d
to
8e51ea8
Compare
Ephemeral environment shutdown and build artifacts deleted. |
SUMMARY
This is helpful to enable some iframing of dashboard use cases. Depending on your domain sharding setup, it may not work across origins, so this is a convenient workaround
TESTING INSTRUCTIONS
CI, manual verification
Note that no unit test was added because this function already is tightly integrated with the document and pulling data from it. This would require quite a large refactor to properly test the existing behavior, much less my newly added behavior
ADDITIONAL INFORMATION
to: @kgabryje @graceguo-supercat @michael-s-molina @ktmud