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.
feat(react-router): Add build-time config #15406
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(react-router): Add build-time config #15406
Changes from 17 commits
c97c324
dca9d06
fc5bfe7
7cc99cd
367bffb
cf61939
5164de0
974f0cb
acbe928
0366d2d
3de3f57
73c80f5
63b504a
c52d220
07cf339
053a47b
6e5c009
006c45b
a30b3e2
066d506
ae60d70
a25b27b
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.
l: just to confirm does this not throw a type error? or is
defineConfig
lenient enough to allow adding arbitrary keys to the config object?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.
Also something to explore in the future: Maybe we can add a plugin in
sentryReactRouter
that adds thesentryConfig
in theconfig
hook to the vite config, so that users don't have to do it explicitly. Not sure if you already tried this, or which instance of the vite config is passed intobuildEnd
but maybe it's worth a shot. we could even try to write it onto the global object to pass it over 😅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.
Didn't throw a type error, but you are right the best solution for this would be to define this in a plugin that we add! Will do that in a follow up task
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.
l/Q: is there some kind of
sequence()
helper or so in case people have more than one buildEnd callback?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.
in this case they could just
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.
m/something to follow up on: We only want to delete source maps by default if it was us who turned on source map generation in the first place. This was the reason why I had to pass the promise for
filesToDeleteAfterUpload
in SvelteKit, because I only knew that oncemakeEnableSourceMapsPlugin
'sconfig
hook was invoked. But since we have the vite config here, and we don't rely on the original file deletion plugin, maybe we can solve this simpler 🤔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 my issue was that I only get the final vite config here and not the initial one, BUT I guess we can maybe just write that into the config as well with a custom plugin (like the sentryConfig from your comment above)