-
-
Couldn't load subscription status.
- Fork 1.7k
feat(react-router): Align options with shared build time options type #18014
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
Conversation
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
3a7fa72 to
b3d44be
Compare
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.
The changes in this file are basically just a back-port of the changes in the bundler-plugin type.
getsentry/sentry-javascript-bundler-plugins@main/packages/bundler-plugin-core/src/types.ts
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.
Looks good!
| } | ||
| // delete sourcemaps after upload | ||
| let updatedFilesToDeleteAfterUpload = sourceMapsUploadOptions?.filesToDeleteAfterUpload; | ||
| let updatedFilesToDeleteAfterUpload = await sourcemaps?.filesToDeleteAfterUpload; |
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.
q: Why is this awaited?
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.
As this can be a Promise when coming from the unstable_... option: getsentry/sentry-javascript-bundler-plugins@4255012
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 am a little late to the party. I leave this here for the record
| ...sentryConfig.sourcemaps, | ||
| ...sourceMapsUploadOptions, | ||
| // eslint-disable-next-line deprecation/deprecation | ||
| disable: sourceMapsUploadOptions?.enabled === false ? true : sentryConfig.sourcemaps?.disable, |
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: following would do the same and is a little shorter
| disable: sourceMapsUploadOptions?.enabled === false ? true : sentryConfig.sourcemaps?.disable, | |
| disable: sourceMapsUploadOptions?.enabled === false || sentryConfig.sourcemaps?.disable, |
|
|
||
| it('should start a span for data requests with active root span', async () => { | ||
| vi.spyOn(Util, 'isDataRequest').mockReturnValue(true); | ||
| // @ts-expect-error MockSpan just for testing |
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: you could use new SentryCore.SentryNonRecordingSpan({ traceId: '1', spanId: '2' }) instead, then you can get rid of the ts-expect-error. But for that you need too adapt the mocking:
vi.mock('@sentry/core', async (importOriginal) => {
return {
...(await importOriginal()),
getActiveSpan: vi.fn(),
...
};
});
closes #17066
Also updates the shared
BuildTimeOptionsBasetype a bit to align with the latest changes.