Skip to content

Commit

Permalink
ref(nextjs): Prework for wrapping data-fetching functions (#5508)
Browse files Browse the repository at this point in the history
This is a number of boring structural changes pulled out of #5503 in order to make it easier to review.

Included changes:
- Create folders for loaders and templates, and move the existing loader and template into their respective new folders.
- Convert the `LoaderThis` type into a generic, so that each loader can specify its own options but share everything specified by webpack.
- Since we are only going to support wrapping for pages written in ESM, make sure that templates are ESM, even if the CJS version of the SDK is being used. This required creating a build script, which in turn required updating our ESLint config.
- Add/clarify comments in existing loader files.
  • Loading branch information
lobsterkatie authored Aug 2, 2022
1 parent 6ea53ed commit 57dee8e
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 18 deletions.
8 changes: 8 additions & 0 deletions packages/nextjs/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,12 @@ module.exports = {
rules: {
'@sentry-internal/sdk/no-async-await': 'off',
},
overrides: [
{
files: ['scripts/**/*.ts'],
parserOptions: {
project: ['../../tsconfig.dev.json'],
},
},
],
};
4 changes: 2 additions & 2 deletions packages/nextjs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@
"scripts": {
"build": "run-p build:rollup build:types",
"build:dev": "run-s build",
"build:rollup": "rollup -c rollup.npm.config.js",
"build:rollup": "ts-node scripts/buildRollup.ts",
"build:types": "tsc -p tsconfig.types.json",
"build:watch": "run-p build:rollup:watch build:types:watch",
"build:dev:watch": "run-s build:watch",
"build:rollup:watch": "rollup -c rollup.npm.config.js --watch",
"build:rollup:watch": "nodemon --ext ts --watch src scripts/buildRollup.ts",
"build:types:watch": "tsc -p tsconfig.types.json --watch",
"build:npm": "ts-node ../../scripts/prepack.ts && npm pack ./build",
"circularDepCheck": "madge --circular src/index.client.ts && madge --circular --exclude 'config/types\\.ts' src/index.server.ts # see https://github.com/pahen/madge/issues/306",
Expand Down
8 changes: 4 additions & 4 deletions packages/nextjs/rollup.npm.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ export default [
),
...makeNPMConfigVariants(
makeBaseNPMConfig({
entrypoints: ['src/config/prefixLoaderTemplate.ts'],
entrypoints: ['src/config/templates/prefixLoaderTemplate.ts'],

packageSpecificConfig: {
output: {
// preserve the original file structure (i.e., so that everything is still relative to `src`)
entryFileNames: 'config/[name].js',
entryFileNames: 'config/templates/[name].js',

// this is going to be add-on code, so it doesn't need the trappings of a full module (and in fact actively
// shouldn't have them, lest they muck with the module to which we're adding it)
Expand All @@ -31,15 +31,15 @@ export default [
),
...makeNPMConfigVariants(
makeBaseNPMConfig({
entrypoints: ['src/config/prefixLoader.ts'],
entrypoints: ['src/config/loaders/prefixLoader.ts'],

packageSpecificConfig: {
output: {
// make it so Rollup calms down about the fact that we're doing `export { loader as default }`
exports: 'default',

// preserve the original file structure (i.e., so that everything is still relative to `src`)
entryFileNames: 'config/[name].js',
entryFileNames: 'config/loaders/[name].js',
},
},
}),
Expand Down
24 changes: 24 additions & 0 deletions packages/nextjs/scripts/buildRollup.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import * as childProcess from 'child_process';
import * as fs from 'fs';
import * as path from 'path';

/**
* Run the given shell command, piping the shell process's `stdin`, `stdout`, and `stderr` to that of the current
* process. Returns contents of `stdout`.
*/
function run(cmd: string, options?: childProcess.ExecSyncOptions): string | Buffer {
return childProcess.execSync(cmd, { stdio: 'inherit', ...options });
}

run('yarn rollup -c rollup.npm.config.js');

// Regardless of whether nextjs is using the CJS or ESM version of our SDK, we want the code from our templates to be in
// ESM (since we'll be adding it onto page files which are themselves written in ESM), so copy the ESM versions of the
// templates over into the CJS build directory. (Building only the ESM version and sticking it in both locations is
// something which in theory Rollup could do, but it would mean refactoring our Rollup helper functions, which isn't
// worth it just for this.)
const cjsTemplateDir = 'build/cjs/config/templates/';
const esmTemplateDir = 'build/esm/config/templates/';
fs.readdirSync(esmTemplateDir).forEach(templateFile =>
fs.copyFileSync(path.join(esmTemplateDir, templateFile), path.join(cjsTemplateDir, templateFile)),
);
Original file line number Diff line number Diff line change
@@ -1,27 +1,22 @@
import * as fs from 'fs';
import * as path from 'path';

import { LoaderThis } from './types';

type LoaderOptions = {
distDir: string;
};
// TODO Use real webpack types
type LoaderThis = {
// Webpack 4
query?: LoaderOptions;
// Webpack 5
getOptions?: () => LoaderOptions;
addDependency: (filepath: string) => void;
};

/**
* Inject templated code into the beginning of a module.
*/
function prefixLoader(this: LoaderThis, userCode: string): string {
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 templatePath = path.resolve(__dirname, 'prefixLoaderTemplate.js');
const templatePath = path.resolve(__dirname, '../templates/prefixLoaderTemplate.js');
// make sure the template is included when runing `webpack watch`
this.addDependency(templatePath);

// Fill in the placeholder
Expand Down
10 changes: 10 additions & 0 deletions packages/nextjs/src/config/loaders/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// TODO Use real webpack types
export type LoaderThis<Options> = {
// Loader options in Webpack 4
query?: Options;
// Loader options in Webpack 5
getOptions?: () => Options;

// Function to add outside file used by loader to `watch` process
addDependency: (filepath: string) => void;
};
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any
(global as any).__rewriteFramesDistDir__ = '__DIST_DIR__';

// We need this to make this file an ESM module, which TS requires when using `isolatedModules`.
// We need this to make this file an ESM module, which TS requires when using `isolatedModules`, but it doesn't affect
// the end result - Rollup recognizes that it's a no-op and doesn't include it when building our code.
export {};
2 changes: 1 addition & 1 deletion packages/nextjs/src/config/webpack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export function constructWebpackConfigFunction(
// Support non-default output directories by making the output path (easy to get here at build-time)
// available to the server SDK's default `RewriteFrames` instance (which needs it at runtime), by
// injecting code to attach it to `global`.
loader: path.resolve(__dirname, 'prefixLoader.js'),
loader: path.resolve(__dirname, 'loaders/prefixLoader.js'),
options: {
distDir: userNextConfig.distDir || '.next',
},
Expand Down

0 comments on commit 57dee8e

Please sign in to comment.