sentry-javascript
sentry-javascript copied to clipboard
feat(nextjs): Wrap server-side getInitialProps
This PR wraps server-side getInitialProps. We do this by creating a virtual proxy module for each next.js pages in our dataFetchersLoader webpack loader that re-exports all the exports of the original pages file but with a wrapped version of getInitiialProps.
The reason we went for the virtual proxy instead of AST modifications, is that in order to wrap getInitialProps we have to cover a lot of edge cases of how 1. people might define a default export 2. define getInitialProps on the default export. The proxy solution allows us to generically cover all of those cases with relatively little effort. Another upside is, that we do not have to mess with user code potentially creating runtime bugs - instead, we only mess with imports/exports which only throw at build time.
The AST modification code that wraps getServerSideProps, getStaticProps, and getStaticPaths currently coexists with the proxy module code. If we deem the proxy code good, we might switch the wrapping of those methods to the proxy approach too.
Additional notes:
- fixed a small bug where not all instances of
__ORIG_FUNC__got replaced - renamed all the wrapping functions because the abbreviations gave me a headache
- "improved" the
loaderThistype (also removed the TODO comment saying we should replace it with webpack types - I don't think we should do this since it's super painful to maintain and implement with our current typescript version which is yet too dumb/slow to do good inference) - I didn't put the injected code in a template since the gain we would have from that doesn't outweigh the additional code complexity
Draft until TODOs are done:
- [x] Write JSDoc
- [ ] Write meaningful comments for what the hell
hasDefaultExportandgetExportIdentifiersare doing - [x] Replace patterns like
declarationNode.id.type === 'ObjectPattern'withObjectPattern.check(declarationNode)
size-limit report 📦
| Path | Size |
|---|---|
| @sentry/browser - ES5 CDN Bundle (gzipped + minified) | 19.39 KB (0%) |
| @sentry/browser - ES5 CDN Bundle (minified) | 59.99 KB (0%) |
| @sentry/browser - ES6 CDN Bundle (gzipped + minified) | 17.96 KB (+0.03% 🔺) |
| @sentry/browser - ES6 CDN Bundle (minified) | 52.88 KB (0%) |
| @sentry/browser - Webpack (gzipped + minified) | 19.73 KB (0%) |
| @sentry/browser - Webpack (minified) | 64.21 KB (0%) |
| @sentry/react - Webpack (gzipped + minified) | 19.75 KB (0%) |
| @sentry/nextjs Client - Webpack (gzipped + minified) | 44.16 KB (+0.01% 🔺) |
| @sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified) | 25.79 KB (+0.02% 🔺) |
| @sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified) | 24.04 KB (0%) |
@lobsterkatie Okay so there was this overarching theme in your comments asking why I prefixed all the function's name with export even though they appear to be generic.
The reason is 🥁... they aren't. The functions only do what they say they're doing when they are inside an export statement. It is entirely possible we encounter ObjectPatterns, ArrayPatterns and RestElements outside of an export statement. In these situations, the functions wouldn't do the right thing though because there are much more edge cases to handle.
On the right-hand side of an assignment you can do stuff like: const asdf = { ...({ a: 1 } || { b: 2 }) }; whereas when you encounter an ObjectPattern on the left-hand side, you can't do that - you're actually super limited in what you are allowed to do.
I added a comment about why you're only allowed to use these functions to extract identifiers only in an export context: aa5b915872d776268bdc71ad6551afde98725561