Skip to content

Commit abebbab

Browse files
lobsterkatieAbhiPrasad
authored andcommitted
fix(build): Ensure bundle builds have needed dependencies (#5119)
In #5094, a change was made to parallelize our repo-level build commands as much as possible. In that PR, it was stated that `build:bundle` could run independent of, and therefore in parallel with, the types and rollup builds, and at the time that was true. When TS (through rollup) builds a bundle, it creates a giant AST out of the bundled package and all of its monorepo dependencies, based on the original source code, then transpiles the whole thing - no prework needed. But in #5111 we switched our ES6 bundles to use sucrase for transpilation (still through rollup), and that changed things. Sucrase (along with every other non-TS transpilation tool) only considers files one by one, not as part of a larger whole, and won't reach across package boundaries, even within the monorepo. As a result, rollup needs all dependencies to already exist in transpiled form, since sucrase doesn't touch them, which becomes a problem if both processes are happening at once. (_But how has CI even been passing since that second PR, then?_, you may ask. The answer is, sucrase is very fast, and lerna can only start so many things at once. It ends up being a race condition between sucrase finishing with the dependencies and lerna kicking off the bundle builds, and almost all the time, sucrase wins. And the other situations in which this is broken all involve using something other than the top-level `build` script to create bundles in a repo with no existing build artifacts, and CI just never does that.) So TL;DR, we need to have already transpiled a packages's monorepo dependencies before that package can be turned into a bundle. For `build:bundle` at both the repo and package level, and for `build` at the package level, this means that if they're not there, we have to build them. For `build` at the repo level, where transpilation of all packages does in fact already happen, we have two options: 1) Push bundle builds to happen alongside `build:extras`, after rollup builds have finished. 2) Mimic what happens when the race condition is successful, but in a way that isn't flaky. In other words, continue to run `build:bundle` in parallel with the types and npm package builds, but guarantee that the needed dependencies have finished building themselves before starting the bundle build. Of the two options, the first is certainly simpler, but it also forces the two longest parts of the build (bundle and types builds) to be sequential, which is the exact _opposite_ of what we want, given that the goal all along has been to make the builds noticeably faster. Choosing the second solution also gives us an excuse to add the extra few lines of code needed to fix the repo-level-build:bundle/package-level-build/package-level-build:bundle problem, which we otherwise probably wouldn't fix. This implements that second strategy, by polling at 5 second intervals for the existence of the needed files, for up to 60 seconds, before beginning the bundle builds. (In practice, it fairly reliably seems to take two retries, or ten seconds, before the bundle build can begin.) It also handles the other three situations above, by building the missing files when necessary. Finally, it fixes a type import to rely on the main types package, rather than a built version of it. This prevents our ES5 bundles (which still use TS for transpilation, in order to also take advantage of its ability to down-compile) from running to the same problem as the sucrase bundles are having.
1 parent 1e8df39 commit abebbab

File tree

9 files changed

+163
-8
lines changed

9 files changed

+163
-8
lines changed

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"private": true,
33
"scripts": {
44
"build": "node ./scripts/verify-packages-versions.js && yarn run-p build:rollup build:types build:bundle && yarn build:extras",
5-
"build:bundle": "lerna run --parallel build:bundle",
5+
"build:bundle": "yarn ts-node scripts/ensure-bundle-deps.ts && yarn lerna run --parallel build:bundle",
66
"build:dev": "run-p build:types build:rollup",
77
"build:extras": "lerna run --parallel build:extras",
88
"build:rollup": "lerna run --parallel build:rollup",

packages/browser/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
},
4545
"scripts": {
4646
"build": "run-p build:rollup build:bundle build:types",
47-
"build:bundle": "rollup --config rollup.bundle.config.js",
47+
"build:bundle": "yarn ts-node ../../scripts/ensure-bundle-deps.ts && yarn rollup --config rollup.bundle.config.js",
4848
"build:dev": "run-p build:rollup build:types",
4949
"build:rollup": "rollup -c rollup.npm.config.js",
5050
"build:types": "tsc -p tsconfig.types.json",

packages/hub/src/session.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
import { Session, SessionContext, SessionStatus } from '@sentry/types';
2-
import { SerializedSession } from '@sentry/types/build/types/session';
1+
import { SerializedSession, Session, SessionContext, SessionStatus } from '@sentry/types';
32
import { dropUndefinedKeys, timestampInSeconds, uuid4 } from '@sentry/utils';
43

54
/**

packages/integrations/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
},
2727
"scripts": {
2828
"build": "run-p build:rollup build:types build:bundle",
29-
"build:bundle": "bash scripts/buildBundles.sh",
29+
"build:bundle": "yarn ts-node ../../scripts/ensure-bundle-deps.ts && bash scripts/buildBundles.sh",
3030
"build:dev": "run-p build:rollup build:types",
3131
"build:rollup": "rollup -c rollup.npm.config.js",
3232
"build:types": "tsc -p tsconfig.types.json",

packages/tracing/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
},
2828
"scripts": {
2929
"build": "run-p build:rollup build:types build:bundle && yarn build:extras #necessary for integration tests",
30-
"build:bundle": "rollup --config rollup.bundle.config.js",
30+
"build:bundle": "yarn ts-node ../../scripts/ensure-bundle-deps.ts && yarn rollup --config rollup.bundle.config.js",
3131
"build:dev": "run-p build:rollup build:types",
3232
"build:extras": "yarn build:prepack",
3333
"build:prepack": "ts-node ../../scripts/prepack.ts --bundles",

packages/types/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ export type {
4646
RequestSession,
4747
RequestSessionStatus,
4848
SessionFlusherLike,
49+
SerializedSession,
4950
} from './session';
5051

5152
// eslint-disable-next-line deprecation/deprecation

packages/vue/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
},
2828
"scripts": {
2929
"build": "run-p build:rollup build:types",
30-
"build:bundle": "rollup --config rollup.bundle.config.js",
30+
"build:bundle": "yarn ts-node ../../scripts/ensure-bundle-deps.ts && yarn rollup --config rollup.bundle.config.js",
3131
"build:dev": "run-p build:rollup build:types",
3232
"build:rollup": "rollup -c rollup.npm.config.js",
3333
"build:types": "tsc -p tsconfig.types.json",

packages/wasm/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
},
3232
"scripts": {
3333
"build": "run-p build:rollup build:bundle build:types",
34-
"build:bundle": "rollup --config rollup.bundle.config.js",
34+
"build:bundle": "yarn ts-node ../../scripts/ensure-bundle-deps.ts && yarn rollup --config rollup.bundle.config.js",
3535
"build:dev": "run-p build:rollup build:types",
3636
"build:rollup": "rollup -c rollup.npm.config.js",
3737
"build:types": "tsc -p tsconfig.types.json",

scripts/ensure-bundle-deps.ts

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
/* eslint-disable no-console */
2+
import * as childProcess from 'child_process';
3+
import * as fs from 'fs';
4+
import * as path from 'path';
5+
import * as util from 'util';
6+
7+
/**
8+
* Ensure that `build:bundle` has all of the dependencies it needs to run. Works at both the repo and package level.
9+
*/
10+
async function ensureBundleBuildPrereqs(options: { dependencies: string[]; maxRetries?: number }): Promise<void> {
11+
const { maxRetries = 12, dependencies } = options;
12+
13+
const {
14+
// The directory in which the yarn command was originally invoked (which won't necessarily be the same as
15+
// `process.cwd()`)
16+
INIT_CWD: yarnInitialDir,
17+
// JSON containing the args passed to `yarn`
18+
npm_config_argv: yarnArgJSON,
19+
} = process.env;
20+
21+
if (!yarnInitialDir || !yarnArgJSON) {
22+
const received = { INIT_CWD: yarnInitialDir, npm_config_argv: yarnArgJSON };
23+
throw new Error(
24+
`Missing environment variables needed for ensuring bundle dependencies. Received:\n${util.inspect(received)}\n`,
25+
);
26+
}
27+
28+
// Did this build get invoked by a repo-level script, or a package-level script, and which script was it?
29+
const isTopLevelBuild = path.basename(yarnInitialDir) === 'sentry-javascript';
30+
const yarnScript = (JSON.parse(yarnArgJSON) as { original: string[] }).original[0];
31+
32+
// convert '@sentry/xyz` to `xyz`
33+
const dependencyDirs = dependencies.map(npmPackageName => npmPackageName.split('/')[1]);
34+
35+
// The second half of the conditional tests if this script is being run by the original top-level command or a
36+
// package-level command spawned by it.
37+
const packagesDir = isTopLevelBuild && yarnInitialDir === process.cwd() ? 'packages' : '..';
38+
39+
if (checkForBundleDeps(packagesDir, dependencyDirs)) {
40+
// We're good, nothing to do, the files we need are there
41+
return;
42+
}
43+
44+
// If we get here, the at least some of the dependencies are missing, but how we handle that depends on how we got
45+
// here. There are six possibilities:
46+
// - We ran `build` or `build:bundle` at the repo level
47+
// - We ran `build` or `build:bundle` at the package level
48+
// - We ran `build` or `build:bundle` at the repo level and lerna then ran `build:bundle` at the package level. (We
49+
// shouldn't ever land here under this scenario - the top-level build should already have handled any missing
50+
// dependencies - but it's helpful to consider all the possibilities.)
51+
//
52+
// In the first version of the first scenario (repo-level `build` -> repo-level `build:bundle`), all we have to do is
53+
// wait, because other parts of `build` are creating them as this check is being done. (Waiting 5 or 10 or even 15
54+
// seconds to start running `build:bundle` in parallel is better than pushing it to the second half of `build`,
55+
// because `build:bundle` is the slowest part of the build and therefore the one we most want to parallelize with
56+
// other slow parts, like `build:types`.)
57+
//
58+
// In all other scenarios, if the dependencies are missing, we have to build them ourselves - with `build:bundle` at
59+
// either level, we're the only thing happening (so no one's going to do it for us), and with package-level `build`,
60+
// types and npm assets are being built simultaneously, but only for the package being bundled, not for its
61+
// dependencies. Either way, it's on us to fix the problem.
62+
//
63+
// TODO: This actually *doesn't* work for package-level `build`, not because of a flaw in this logic, but because
64+
// `build:rollup` has similar dependency needs (it needs types rather than npm builds). We should do something like
65+
// this for that at some point.
66+
67+
if (isTopLevelBuild && yarnScript === 'build') {
68+
let retries = 0;
69+
70+
console.log('\nSearching for bundle dependencies...');
71+
72+
while (retries < maxRetries && !checkForBundleDeps(packagesDir, dependencyDirs)) {
73+
console.log('Bundle dependencies not found. Trying again in 5 seconds.');
74+
retries += 1;
75+
await sleep(5000);
76+
}
77+
78+
if (retries === maxRetries) {
79+
throw new Error(
80+
`\nERROR: \`yarn build:bundle\` (triggered by \`yarn build\`) cannot find its depdendencies, despite waiting ${
81+
5 * maxRetries
82+
} seconds for the rest of \`yarn build\` to create them. Something is wrong - it shouldn't take that long. Exiting.`,
83+
);
84+
}
85+
86+
console.log(`\nFound all bundle dependencies after ${retries} retries. Beginning bundle build...`);
87+
}
88+
89+
// top-level `build:bundle`, package-level `build` and `build:bundle`
90+
else {
91+
console.warn('\nWARNING: Missing dependencies for bundle build. They will be built before continuing.');
92+
93+
for (const dependencyDir of dependencyDirs) {
94+
console.log(`\nBuilding \`${dependencyDir}\` package...`);
95+
run('yarn build:rollup', { cwd: `${packagesDir}/${dependencyDir}` });
96+
}
97+
98+
console.log('\nAll dependencies built successfully. Beginning bundle build...');
99+
}
100+
}
101+
102+
/**
103+
* See if all of the necessary dependencies exist
104+
*/
105+
function checkForBundleDeps(packagesDir: string, dependencyDirs: string[]): boolean {
106+
for (const dependencyDir of dependencyDirs) {
107+
const depBuildDir = `${packagesDir}/${dependencyDir}/build`;
108+
109+
// Checking that the directories exist isn't 100% the same as checking that the files themselves exist, of course,
110+
// but it's a decent proxy, and much simpler to do than checking for individual files.
111+
if (
112+
!(
113+
(fs.existsSync(`${depBuildDir}/cjs`) && fs.existsSync(`${depBuildDir}/esm`)) ||
114+
(fs.existsSync(`${depBuildDir}/npm/cjs`) && fs.existsSync(`${depBuildDir}/npm/esm`))
115+
)
116+
) {
117+
// Fail fast
118+
return false;
119+
}
120+
}
121+
122+
return true;
123+
}
124+
125+
/**
126+
* Wait the given number of milliseconds before continuing.
127+
*/
128+
async function sleep(ms: number): Promise<void> {
129+
await new Promise(resolve =>
130+
setTimeout(() => {
131+
resolve();
132+
}, ms),
133+
);
134+
}
135+
136+
/**
137+
* Run the given shell command, piping the shell process's `stdin`, `stdout`, and `stderr` to that of the current
138+
* process. Returns contents of `stdout`.
139+
*/
140+
function run(cmd: string, options?: childProcess.ExecSyncOptions): string {
141+
return String(childProcess.execSync(cmd, { stdio: 'inherit', ...options }));
142+
}
143+
144+
// TODO: Not ideal that we're hard-coding this, and it's easy to get when we're in a package directory, but would take
145+
// more work to get from the repo level. Fortunately this list is unlikely to change very often, and we're the only ones
146+
// we'll break if it gets out of date.
147+
const dependencies = ['@sentry/utils', '@sentry/hub', '@sentry/core'];
148+
149+
if (['sentry-javascript', 'tracing', 'wasm'].includes(path.basename(process.cwd()))) {
150+
dependencies.push('@sentry/browser');
151+
}
152+
153+
void ensureBundleBuildPrereqs({
154+
dependencies,
155+
});

0 commit comments

Comments
 (0)