Skip to content
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(nextjs): Wrap server-side getInitialProps #5546

Merged
merged 25 commits into from
Aug 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
ef89101
feat(nextjs): Wrap server-side getInitialProps
lforst Aug 9, 2022
56aa62e
Add type guards
lforst Aug 9, 2022
a081582
Add jsdoc
lforst Aug 9, 2022
916fdb0
Apply comment wording suggestions
lforst Aug 10, 2022
baa0a5a
Simplify and speed up hasDefaultExport
lforst Aug 10, 2022
145ed92
Clean up `getExportIdentifiersFromRestElement` description
lforst Aug 10, 2022
ad46cdb
Sorted imports
lforst Aug 10, 2022
460f113
Simplify `getExportIdentifiersFromRestElement`
lforst Aug 10, 2022
0002873
Clean up `getExportIdentifiersFromArrayPattern` description
lforst Aug 10, 2022
aef9635
Rename `getExportIdentifiers` to `getExportIdentifierNames`
lforst Aug 10, 2022
41ebd28
Rename `namedExportDeclarationNodes` to `namedExportDeclarationNodeDe…
lforst Aug 10, 2022
0f1390b
Add comments and eliminate dead code paths
lforst Aug 10, 2022
36f4434
s/toStrictEqual/toBe/ in test
lforst Aug 10, 2022
cd94b12
Add name to anonymous function for better debugging
lforst Aug 10, 2022
7f0c91b
Remove unnecessary eslint ignores
lforst Aug 10, 2022
4c97ee9
Preserve enumerable property
lforst Aug 10, 2022
3ccbe10
Add comments clarifying why getInitialProps wrapper is imported directly
lforst Aug 10, 2022
4c3c3ce
Add comment explaining wht proxying is
lforst Aug 10, 2022
c0577da
Remove defineProperty call
lforst Aug 10, 2022
ed15ca3
Pull undefined check out of wrapper
lforst Aug 10, 2022
2f88743
Remove redundant comments
lforst Aug 10, 2022
adad5e3
Remove redundant comments
lforst Aug 10, 2022
aa5b915
Add disclaimiers why functions can only be used in an export context
lforst Aug 10, 2022
b7fc5a6
Explain `getExportIdentifiersFromObjectPattern`
lforst Aug 10, 2022
62bba32
Remove unnecessary `getIdentifierFromRestElement`
lforst Aug 11, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
211 changes: 209 additions & 2 deletions packages/nextjs/src/config/loaders/ast.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,24 @@ const jscs = jscodeshiftDefault || jscodeshiftNamespace;

// These are types not in the TS sense, but in the instance-of-a-Type-class sense
const {
ArrayPattern,
ClassDeclaration,
ExportAllDeclaration,
ExportDefaultDeclaration,
ExportDefaultSpecifier,
ExportNamedDeclaration,
ExportSpecifier,
FunctionDeclaration,
Identifier,
ImportSpecifier,
JSXIdentifier,
MemberExpression,
Node,
ObjectExpression,
ObjectPattern,
Property,
RestElement,
TSTypeParameter,
VariableDeclaration,
VariableDeclarator,
} = jscs;
Expand Down Expand Up @@ -291,8 +301,6 @@ function maybeRenameNode(ast: AST, identifierPath: ASTPath<IdentifierNode>, alia
// it means we potentially need to rename something *not* already named `getServerSideProps`, `getStaticProps`, or
// `getStaticPaths`, meaning we need to rename nodes outside of the collection upon which we're currently acting.
if (ExportSpecifier.check(parent)) {
// console.log(node);
// debugger;
if (parent.exported.name !== parent.local?.name && node === parent.exported) {
lforst marked this conversation as resolved.
Show resolved Hide resolved
const currentLocalName = parent.local?.name || '';
renameIdentifiers(ast, currentLocalName, alias);
Expand Down Expand Up @@ -320,3 +328,202 @@ export function removeComments(ast: AST): void {
const nodesWithComments = ast.find(Node).filter(nodePath => !!nodePath.node.comments);
nodesWithComments.forEach(nodePath => (nodePath.node.comments = null));
}

/**
* Determines from a given AST of a file whether the file has a default export or not.
*/
export function hasDefaultExport(ast: AST): boolean {
const defaultExports = ast.find(Node, value => {
return (
ExportDefaultDeclaration.check(value) ||
ExportDefaultSpecifier.check(value) ||
(ExportSpecifier.check(value) && value.exported.name === 'default')
);
});

// In theory there should only ever be 0 or 1, but who knows what people do
return defaultExports.length > 0;
}

/**
* Extracts all identifier names (`'constName'`) from an destructuringassignment'sArrayPattern (the `[constName]` in`const [constName] = [1]`).
*
* This function recursively calls itself and `getExportIdentifiersFromObjectPattern` since destructuring assignments
* can be deeply nested with objects and arrays.
*
* Example - take the following program:
*
* ```js
* export const [{ foo: name1 }, [{ bar: [name2]}, name3]] = [{ foo: 1 }, [{ bar: [2] }, 3]];
lforst marked this conversation as resolved.
Show resolved Hide resolved
* ```
*
* The `ArrayPattern` node in question for this program is the left hand side of the assignment:
* `[{ foo: name1 }, [{ bar: [name2]}, name3]]`
*
* Applying this function to this `ArrayPattern` will return the following: `["name1", "name2", "name3"]`
*
* DISCLAIMER: This function only correcly extracts identifiers of `ArrayPatterns` in the context of export statements.
* Using this for `ArrayPattern` outside of exports would require us to handle more edgecases. Hence the "Export" in
* this function's name.
*/
function getExportIdentifiersFromArrayPattern(arrayPattern: jscsTypes.ArrayPattern): string[] {
const identifiers: string[] = [];

arrayPattern.elements.forEach(element => {
if (Identifier.check(element)) {
identifiers.push(element.name);
} else if (ObjectPattern.check(element)) {
identifiers.push(...getExportIdentifiersFromObjectPattern(element));
lforst marked this conversation as resolved.
Show resolved Hide resolved
} else if (ArrayPattern.check(element)) {
identifiers.push(...getExportIdentifiersFromArrayPattern(element));
} else if (RestElement.check(element) && Identifier.check(element.argument)) {
// `RestElements` are spread operators
identifiers.push(element.argument.name);
}
});

return identifiers;
}

/**
* Grabs all identifiers from an ObjectPattern within a destructured named export declaration
* statement (`name` in "export const { val: name } = { val: 1 }").
*
* This function recursively calls itself and `getExportIdentifiersFromArrayPattern` since destructuring assignments
* can be deeply nested with objects and arrays.
*
* Example - take the following program:
*
* ```js
* export const { foo: [{ bar: name1 }], name2, ...name3 } = { foo: [{}] };
* ```
*
* The `ObjectPattern` node in question for this program is the left hand side of the assignment:
* `{ foo: [{ bar: name1 }], name2, ...name3 } = { foo: [{}] }`
*
* Applying this function to this `ObjectPattern` will return the following: `["name1", "name2", "name3"]`
*
* DISCLAIMER: This function only correcly extracts identifiers of `ObjectPatterns` in the context of export statements.
* Using this for `ObjectPatterns` outside of exports would require us to handle more edgecases. Hence the "Export" in
* this function's name.
*/
function getExportIdentifiersFromObjectPattern(objectPatternNode: jscsTypes.ObjectPattern): string[] {
const identifiers: string[] = [];

objectPatternNode.properties.forEach(property => {
// An `ObjectPattern`'s properties can be either `Property`s or `RestElement`s.
if (Property.check(property)) {
lobsterkatie marked this conversation as resolved.
Show resolved Hide resolved
if (Identifier.check(property.value)) {
identifiers.push(property.value.name);
} else if (ObjectPattern.check(property.value)) {
identifiers.push(...getExportIdentifiersFromObjectPattern(property.value));
} else if (ArrayPattern.check(property.value)) {
identifiers.push(...getExportIdentifiersFromArrayPattern(property.value));
} else if (RestElement.check(property.value) && Identifier.check(property.value.argument)) {
// `RestElements` are spread operators
identifiers.push(property.value.argument.name);
}
// @ts-ignore AST types are wrong here
} else if (RestElement.check(property) && Identifier.check(property.argument)) {
// `RestElements` are spread operators
// @ts-ignore AST types are wrong here
identifiers.push(property.argument.name as string);
}
});

return identifiers;
}

/**
* Given the AST of a file, this function extracts all named exports from the file.
*
* @returns a list of deduplicated identifiers.
*/
export function getExportIdentifierNames(ast: AST): string[] {
// We'll use a set to dedupe at the end, but for now we use an array as our accumulator because you can add multiple elements to it at once.
const identifiers: string[] = [];
lobsterkatie marked this conversation as resolved.
Show resolved Hide resolved

// The following variable collects all export statements that double as named declaration, e.g.:
// - export function myFunc() {}
// - export var myVar = 1337
// - export const myConst = 1337
// - export const { a, ..rest } = { a: 1, b: 2, c: 3 }
// We will narrow those situations down in subsequent code blocks.
const namedExportDeclarationNodeDeclarations = ast
.find(ExportNamedDeclaration)
.nodes()
.map(namedExportDeclarationNode => namedExportDeclarationNode.declaration);

namedExportDeclarationNodeDeclarations
.filter((declarationNode): declarationNode is jscsTypes.VariableDeclaration =>
// Narrow down to varible declarations, e.g.:
// export const a = ...;
// export var b = ...;
// export let c = ...;
// export let c, d = 1;
VariableDeclaration.check(declarationNode),
)
.map(
variableDeclarationNode =>
// Grab all declarations in a single export statement.
// There can be multiple in the case of for example in `export let a, b;`.
variableDeclarationNode.declarations,
)
.reduce((prev, curr) => [...prev, ...curr], []) // flatten - now we have all declaration nodes in one flat array
.forEach(declarationNode => {
if (
Identifier.check(declarationNode) || // should never happen
JSXIdentifier.check(declarationNode) || // JSX like `<name></name>` - we don't care about these
TSTypeParameter.check(declarationNode) // type definitions - we don't care about those
) {
// We should never have to enter this branch, it is just for type narrowing.
} else if (Identifier.check(declarationNode.id)) {
// If it's a simple declaration with an identifier we collect it. (e.g. `const myIdentifier = 1;` -> "myIdentifier")
identifiers.push(declarationNode.id.name);
} else if (ObjectPattern.check(declarationNode.id)) {
// If we encounter a destructuring export like `export const { foo: name1, bar: name2 } = { foo: 1, bar: 2 };`,
// we try collecting the identifiers from the pattern `{ foo: name1, bar: name2 }`.
identifiers.push(...getExportIdentifiersFromObjectPattern(declarationNode.id));
} else if (ArrayPattern.check(declarationNode.id)) {
// If we encounter a destructuring export like `export const [name1, name2] = [1, 2];`,
// we try collecting the identifiers from the pattern `[name1, name2]`.
identifiers.push(...getExportIdentifiersFromArrayPattern(declarationNode.id));
}
});

namedExportDeclarationNodeDeclarations
.filter(
// Narrow down to class and function declarations, e.g.:
// export class Foo {};
// export function bar() {};
(declarationNode): declarationNode is jscsTypes.ClassDeclaration | jscsTypes.FunctionDeclaration =>
ClassDeclaration.check(declarationNode) || FunctionDeclaration.check(declarationNode),
)
.map(node => node.id) // Grab the identifier of the function/class - Note: it might be `null` when it's anonymous
.filter((id): id is jscsTypes.Identifier => Identifier.check(id)) // Elaborate way of null-checking
.forEach(id => identifiers.push(id.name)); // Collect the name of the identifier

ast
.find(ExportSpecifier) // Find stuff like `export {<id [as name]>} [from ...];`
.nodes()
.forEach(specifier => {
// Taking the example above `specifier.exported.name` always contains `id` unless `name` is specified, then it's `name`;
if (specifier.exported.name !== 'default') {
// You can do default exports "export { something as default };" but we do not want to collect "default" in this
// function since it only wants to collect named exports.
identifiers.push(specifier.exported.name);
}
});

ast
.find(ExportAllDeclaration) // Find stuff like `export * from ..." and "export * as someVariable from ...`
.nodes()
.forEach(declaration => {
// Narrow it down to only find `export * as someVariable from ...` (emphasis on "as someVariable")
if (declaration.exported) {
identifiers.push(declaration.exported.name); // `declaration.exported.name` contains "someVariable"
}
});

return [...new Set(identifiers)]; // dedupe
}
112 changes: 80 additions & 32 deletions packages/nextjs/src/config/loaders/dataFetchersLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,26 @@
* manipulating them, and then turning them back into strings and appending our template code to the user's (modified)
* page code. Greater detail and explanations can be found in situ in the functions below and in the helper functions in
* `ast.ts`.
*
* For `getInitialProps` we create a virtual proxy-module that re-exports all the exports and default exports of the
* original file and wraps `getInitialProps`. We do this since it allows us to very generically wrap `getInitialProps`
* for all kinds ways users might define default exports (which are a lot of ways).
*/

import { logger } from '@sentry/utils';
import * as fs from 'fs';
import * as path from 'path';

import { isESM } from '../../utils/isESM';
import type { AST } from './ast';
import { findDeclarations, findExports, makeAST, removeComments, renameIdentifiers } from './ast';
import {
findDeclarations,
findExports,
getExportIdentifierNames,
hasDefaultExport,
makeAST,
removeComments,
renameIdentifiers,
} from './ast';
import type { LoaderThis } from './types';

// Map to keep track of each function's placeholder in the template and what it should be replaced with. (The latter
Expand Down Expand Up @@ -94,44 +105,81 @@ function wrapFunctions(userCode: string, templateCode: string, filepath: string)
* Wrap `getStaticPaths`, `getStaticProps`, and `getServerSideProps` (if they exist) in the given page code
*/
export default function wrapDataFetchersLoader(this: LoaderThis<LoaderOptions>, userCode: string): string {
// We know one or the other will be defined, depending on the version of webpack being used
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const { projectDir } = this.getOptions ? this.getOptions() : this.query!;

// For now this loader only works for ESM code
if (!isESM(userCode)) {
return userCode;
}

// If none of the functions we want to wrap appears in the page's code, there's nothing to do. (Note: We do this as a
// simple substring match (rather than waiting until we've parsed the code) because it's meant to be an
// as-fast-as-possible fail-fast. It's possible for user code to pass this check, even if it contains none of the
// functions in question, just by virtue of the correct string having been found, be it in a comment, as part of a
// longer variable name, etc. That said, when we actually do the code manipulation we'll be working on the code's AST,
// meaning we'll be able to differentiate between code we actually want to change and any false positives which might
// come up here.)
if (Object.keys(DATA_FETCHING_FUNCTIONS).every(functionName => !userCode.includes(functionName))) {
return userCode;
}
// We know one or the other will be defined, depending on the version of webpack being used
const { projectDir } = 'getOptions' in this ? this.getOptions() : this.query;

const templatePath = path.resolve(__dirname, '../templates/dataFetchersLoaderTemplate.js');
// make sure the template is included when runing `webpack watch`
this.addDependency(templatePath);
// In the following branch we will proxy the user's file. This means we return code (basically an entirely new file)
// that re - exports all the user file's originial export, but with a "sentry-proxy-loader" query in the module
// string.
// This looks like the following: `export { a, b, c } from "[imagine userfile path here]?sentry-proxy-loader";`
// Additionally, in this proxy file we import the userfile's default export, wrap `getInitialProps` on that default
// export, and re -export the now modified default export as default.
// Webpack will resolve the module with the "sentry-proxy-loader" query to the original file, but will give us access
// to the query via`this.resourceQuery`. If we see that `this.resourceQuery` includes includes "sentry-proxy-loader"
// we know we're in a proxied file and do not need to proxy again.

const templateCode = fs.readFileSync(templatePath).toString();
if (!this.resourceQuery.includes('sentry-proxy-loader')) {
lforst marked this conversation as resolved.
Show resolved Hide resolved
const ast = makeAST(userCode, true); // is there a reason to ever parse without typescript?
Copy link
Member

Choose a reason for hiding this comment

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

// is there a reason to ever parse without typescript?

Have you tried parsing a JS file with the tsx parser? Does it work? If so, then no, probably not.

Copy link
Member Author

Choose a reason for hiding this comment

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

The "files" (strings) we're parsing in the ast.test.ts tests are JS and JS is a subset of TS so I am pretty positive it would work. Should we remove the differentiation and just always parse with typescript? Wdyt?

Copy link
Member Author

Choose a reason for hiding this comment

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

Discussed: Let's try a big JS file or two and see if it errs out. If it works lets simplify the makeAst logic.

Copy link
Member

Choose a reason for hiding this comment

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

Done in #5563.


const [modifiedUserCode, modifiedTemplateCode] = wrapFunctions(
userCode,
templateCode,
// Relative path to the page we're currently processing, for use in error messages
path.relative(projectDir, this.resourcePath),
);
const exportedIdentifiers = getExportIdentifierNames(ast);

// Fill in template placeholders
let injectedCode = modifiedTemplateCode;
for (const { placeholder, alias } of Object.values(DATA_FETCHING_FUNCTIONS)) {
injectedCode = injectedCode.replace(placeholder, alias);
}
let outputFileContent = '';

if (exportedIdentifiers.length > 0) {
outputFileContent += `export { ${exportedIdentifiers.join(', ')} } from "${
this.resourcePath
}?sentry-proxy-loader";`;
}

if (hasDefaultExport(ast)) {
lobsterkatie marked this conversation as resolved.
Show resolved Hide resolved
outputFileContent += `
import { default as _sentry_default } from "${this.resourcePath}?sentry-proxy-loader";
import { withSentryGetInitialProps } from "@sentry/nextjs";

return `${modifiedUserCode}\n${injectedCode}`;
if (typeof _sentry_default.getInitialProps === 'function') {
_sentry_default.getInitialProps = withSentryGetInitialProps(_sentry_default.getInitialProps);
}

export default _sentry_default;`;
}

return outputFileContent;
} else {
// If none of the functions we want to wrap appears in the page's code, there's nothing to do. (Note: We do this as a
// simple substring match (rather than waiting until we've parsed the code) because it's meant to be an
// as-fast-as-possible fail-fast. It's possible for user code to pass this check, even if it contains none of the
// functions in question, just by virtue of the correct string having been found, be it in a comment, as part of a
// longer variable name, etc. That said, when we actually do the code manipulation we'll be working on the code's AST,
// meaning we'll be able to differentiate between code we actually want to change and any false positives which might
// come up here.)
if (Object.keys(DATA_FETCHING_FUNCTIONS).every(functionName => !userCode.includes(functionName))) {
return userCode;
}

const templatePath = path.resolve(__dirname, '../templates/dataFetchersLoaderTemplate.js');
// make sure the template is included when runing `webpack watch`
this.addDependency(templatePath);

const templateCode = fs.readFileSync(templatePath).toString();

const [modifiedUserCode, modifiedTemplateCode] = wrapFunctions(
userCode,
templateCode,
// Relative path to the page we're currently processing, for use in error messages
path.relative(projectDir, this.resourcePath),
);

// Fill in template placeholders
let injectedCode = modifiedTemplateCode;
for (const { placeholder, alias } of Object.values(DATA_FETCHING_FUNCTIONS)) {
injectedCode = injectedCode.replace(new RegExp(placeholder, 'g'), alias);
}

return `${modifiedUserCode}\n${injectedCode}`;
}
}
3 changes: 1 addition & 2 deletions packages/nextjs/src/config/loaders/prefixLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ type LoaderOptions = {
*/
export default function prefixLoader(this: LoaderThis<LoaderOptions>, userCode: string): string {
// We know one or the other will be defined, depending on the version of webpack being used
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const { distDir } = this.getOptions ? this.getOptions() : this.query!;
const { distDir } = 'getOptions' in this ? this.getOptions() : this.query;
lforst marked this conversation as resolved.
Show resolved Hide resolved

const templatePath = path.resolve(__dirname, '../templates/prefixLoaderTemplate.js');
// make sure the template is included when runing `webpack watch`
Expand Down
Loading