Skip to content

Conversation

@lforst
Copy link
Contributor

@lforst lforst commented Feb 14, 2023

This came up in discord: https://discord.com/channels/621778831602221064/621786575591702529/1074841610753413261

Screenshot 2023-02-14 at 13 41 34

Our Next.js types currently have references to the Node package but the paths in the reference contain our internal build structure which leads to mismatches when you use the package outside of our monorepo.

https://unpkg.com/browse/@sentry/[email protected]/types/index.types.d.ts

I didn't know how to fix this properly via settings and stuff but slightly changing how we import things seemed to get rid of these import() statements.

Also added an import that would have failed to our E2E tests.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 14, 2023

Replay SDK metrics 🚀

    Plain +Sentry +Replay
Revision Value Value Diff Ratio Value Diff Ratio
LCP This PR a993786 118.42 ms 125.10 ms +6.68 ms +5.64 % 176.16 ms +57.74 ms +48.76 %
Previous e9eec27 67.47 ms 102.63 ms +35.17 ms +52.13 % 128.85 ms +61.38 ms +90.98 %
CLS This PR a993786 0.06 ms 0.06 ms -0.00 ms -0.03 % 0.06 ms -0.00 ms -0.33 %
Previous e9eec27 0.06 ms 0.06 ms +0.00 ms +0.07 % 0.06 ms -0.00 ms -0.33 %
CPU This PR a993786 27.13 % 28.59 % +1.46 pp +5.37 % 36.67 % +9.54 pp +35.15 %
Previous e9eec27 19.40 % 19.02 % -0.37 pp -1.92 % 25.44 % +6.04 pp +31.16 %
JS heap avg This PR a993786 1.93 MB 1.99 MB +63.78 kB +3.30 % 2.86 MB +931.83 kB +48.28 %
Previous e9eec27 1.94 MB 1.99 MB +45.82 kB +2.36 % 2.87 MB +927.84 kB +47.75 %
JS heap max This PR a993786 2.32 MB 2.59 MB +266.27 kB +11.48 % 3.36 MB +1.04 MB +44.94 %
Previous e9eec27 2.3 MB 2.55 MB +249.12 kB +10.82 % 3.35 MB +1.05 MB +45.46 %
netTx This PR a993786 0 B 0 B 0 B n/a 2.21 kB +2.21 kB n/a
Previous e9eec27 0 B 0 B 0 B n/a 2.21 kB +2.21 kB n/a
netRx This PR a993786 0 B 0 B 0 B n/a 41 B +41 B n/a
Previous e9eec27 0 B 0 B 0 B n/a 41 B +41 B n/a
netCount This PR a993786 0 0 0 n/a 1 +1 n/a
Previous e9eec27 0 0 0 n/a 1 +1 n/a
netTime This PR a993786 0.00 ms 0.00 ms 0.00 ms n/a 109.41 ms +109.41 ms n/a
Previous e9eec27 0.00 ms 0.00 ms 0.00 ms n/a 88.58 ms +88.58 ms n/a

Previous results on branch: develop

RevisionLCPCLSCPUJS heap avgJS heap maxnetTxnetRxnetCountnetTime
e9eec27+61.38 ms-0.00 ms+6.04 pp+927.84 kB+1.05 MB+2.21 kB+41 B+1+88.58 ms
d604022+58.83 ms-0.00 ms+7.65 pp+930.16 kB+1.05 MB+2.21 kB+41 B+1+109.63 ms
a961e57+54.75 ms-0.00 ms+6.50 pp+929.18 kB+1.07 MB+2.21 kB+41 B+1+92.73 ms
f7c0a2f+46.14 ms+0.00 ms+6.37 pp+921.47 kB+1.06 MB+2.23 kB+41 B+1+207.30 ms
cb19818+57.16 ms+0.00 ms+11.95 pp+1.07 MB+2.21 MB+2.52 kB+41 B+1+111.50 ms
ee301c3+71.07 ms-0.00 ms+12.64 pp+1.07 MB+2.22 MB+2.55 kB+41 B+1+94.67 ms
93c4759+61.10 ms-0.00 ms+12.72 pp+1.08 MB+2.19 MB+2.57 kB+41 B+1+116.75 ms
274f489+63.60 ms-0.00 ms+11.56 pp+1.08 MB+2.2 MB+2.56 kB+41 B+1+116.60 ms
4827b60+58.67 ms+0.00 ms+18.38 pp+1.07 MB+2.22 MB+2.6 kB+41 B+1+91.21 ms
c3806eb+79.85 ms-0.00 ms+12.10 pp+1.05 MB+2.16 MB+2.54 kB+41 B+1+93.58 ms

*) pp - percentage points - an absolute difference between two percentages.
Last updated: Tue, 14 Feb 2023 13:16:46 GMT

@github-actions
Copy link
Contributor

github-actions bot commented Feb 14, 2023

size-limit report 📦

Path Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified) 20.09 KB (+0.06% 🔺)
@sentry/browser - ES5 CDN Bundle (minified) 62.23 KB (+0.03% 🔺)
@sentry/browser - ES6 CDN Bundle (gzipped + minified) 18.71 KB (+0.06% 🔺)
@sentry/browser - ES6 CDN Bundle (minified) 55.37 KB (+0.04% 🔺)
@sentry/browser - Webpack (gzipped + minified) 20.44 KB (+0.03% 🔺)
@sentry/browser - Webpack (minified) 66.81 KB (+0.03% 🔺)
@sentry/react - Webpack (gzipped + minified) 20.47 KB (+0.03% 🔺)
@sentry/nextjs Client - Webpack (gzipped + minified) 47.9 KB (+0.02% 🔺)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified) 27.03 KB (+0.04% 🔺)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified) 25.29 KB (+0.06% 🔺)
@sentry/replay ES6 CDN Bundle (gzipped + minified) 42.51 KB (+0.06% 🔺)
@sentry/replay - Webpack (gzipped + minified) 36.67 KB (-0.6% 🔽)
@sentry/browser + @sentry/tracing + @sentry/replay - ES6 CDN Bundle (gzipped + minified) 60.23 KB (+0.04% 🔺)
@sentry/browser + @sentry/replay - ES6 CDN Bundle (gzipped + minified) 53.77 KB (+0.05% 🔺)

@lforst lforst requested review from AbhiPrasad and mydea February 14, 2023 12:50
@lforst
Copy link
Contributor Author

lforst commented Feb 14, 2023

This is all just a big learning process. Will come in handy when tackling more full-stack frameworks.

@lforst lforst marked this pull request as ready for review February 14, 2023 12:51
import { isBuild } from './utils/isBuild';

export * from '@sentry/node';
export { Integrations };
Copy link
Member

Choose a reason for hiding this comment

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

why do we have to explicitly export, doesn't the wildcard above take care of this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah wait I think this is a leftover from messing around... Lemme check if I can remove it or if it breaks again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, wasn't needed --> I removed it.

Luca Forstner and others added 2 commits February 14, 2023 13:03
Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

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

Makes sense. I wonder if we should lint for this 🤔

@lforst
Copy link
Contributor Author

lforst commented Feb 14, 2023

Makes sense. I wonder if we should lint for this 🤔

Linting is already pretty strong here - ts does a somewhat good job. The weird thing was just the build output...

One thing I had in mind once was having snapshot tests for our type definitions. Every time they change, we should have to sign off in order to ensure no unintended breakage.

@lforst lforst merged commit 1921888 into develop Feb 14, 2023
@lforst lforst deleted the lforst-fix-nextjs-integrations-types branch February 14, 2023 13:25
ramchaik pushed a commit to ramchaik/sentry-javascript that referenced this pull request Feb 14, 2023
@AbhiPrasad
Copy link
Member

I like the idea of snapshot tests! Let's open a ticket?

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.

3 participants