ref(build): Build types separately #4724
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
size-limit report
|
lforst
requested changes
Mar 17, 2022
lobsterkatie
force-pushed
the
kmclb-build-types-separately
branch
from
4fdb16d
to
c982773
Compare
…uild:watch`, and `build:dev:watch` commands
lobsterkatie
force-pushed
the
kmclb-build-types-separately
branch
from
c982773
to
e84ffa8
Compare
lforst
approved these changes
Mar 18, 2022
Thanks for helping me clean it up! Teamwork FTW! 🙂 |
|
In @sentry/node v6.19.0, the Could you please inquire? |
3 tasks
lobsterkatie
added a commit
that referenced
this pull request
Mar 22, 2022
A few of the `build:types:watch` instances added in #4724 got clobbered in a subsequent rebase. This adds the missing ones back in.
The problem that caused should be solved in 6.19.1. Please let us know if upgrade doesn't fix it for you. |
It did fix it, thanks. |
Lms24
added a commit
that referenced
this pull request
Mar 31, 2022
…ints (#4824) * All tarballs now include the `build/types` directory. All `.npmignore`s were adjusted to include this directory. * The `types` entry point in all `package.json`s are now adjusted to the new types directory Proper bugfix for 6.19.0 regression: With PR #4724, we introduced separate building of type declaration files which would be written to `<sdk>/build/types`. Furthermore, we adjusted the `types` entry point in the `package.json`s to this new directory. To avoid a breaking change, we kept type declarations in `dist` and `esm`. However, a bug in this PR caused a small regression in most of our SDKs: The new types directory was not included in most of our SDK tarballs. Subsequently, we released a hot fix via #4745 which simply reset the entry points back to their previous path (`dist/index.d.ts`) in all SDKs. Additional adjustments w.r.t. creating tarballs to improve consistency: * Browser: Adjusted `postbuild.sh` to copy `.npmignore` to `./build` * Gatsby: Use `.npmignore` instead of `files` property in `package.json` * Utils: Changed `.npmignore` to be more similar to our other packages * WASM: Changed `.npmignore` to be more similar to our other packages * Next.js: Added `.npmignore`
lobsterkatie
added a commit
that referenced
this pull request
May 10, 2022
This switches our `build` and `build:dev` yarn scripts to use the new rollup/sucrase build process. This is the culmination of a number of previous changes, whose highlights include: - #4724, which made building types files a separate build process - #4895, which updated the SDK to use TS 3.8.3 - #4926, which removed use of the `Severity` enum - #5005, which switch our default tsconfig to target es6 - #4992, which added the Sucrase plugin, some helper functions, and the `yarn build:rollup` script - #4993, which added rollup plugins to use `var` rather than `const` and clean up the built code in various ways - #5022, which applied the same `const`-to-`var` translation to tests - #5023, which added the ability to change injected polyfills into imports The result is that, as of this PR, we will no longer use `tsc` to transpile or down-complile our code when building npm packages. Instead, we will be using Rollup to handle making our code CJS-friendlly and Sucrase to handle the transpilation from TS to JS. The main advantages of this change are: - It forced us to do a lot of updating, centralizing, and cleanup of our tooling, not just for build but also for testing and linting. - It brings our CDN build process and our npm build process much more in line with each other, for easier maintainability. - It gives us more control over the eventual output, because we have access to a whole internet full of Rollup plugins (not to mention the ability to make our own), rather than being constrained to tsconfig options. (Plugins also allow us to interact with the code directly.) - It speeds up our builds fairly significantly. I ran a number of trials in GHA of running `yarn build:dev` at the top level of the repo. Before this change, the average time was ~150 seconds. After this change, it's about half that, roughly 75 seconds. Because of the switch in tooling, the code we publish is going to be slightly different. In order to make sure that those differences weren't going to be breaking, I built each package under the old system and under the new system, ran a `git diff`, and checked every file, both CJS and ESM, in every package affected by this change. The differences (none of which affect behavior or eventual bundle size by more than a few bytes in each direction), fell into a few categories: - Purely cosmetic changes, things like which comments are retained, the order of imports, where in the file exports live, etc. - Changes to class constructors, things like not explicitly assigning `undefined` to undefined attributes, using regular assignment rather than `Object.defineProperty` for attributes which are assigned values, and splitting some of those assignments off into helper functions. - Changes related to the upgrade to ES6 and dropping of support for Node 6, things like not polyfilling object spread or async/await While this represents the most significant part of the overall change, a few outstanding tasks remain: - Making this same switch in `build:watch` - Parallelizing the builds, both locally and in CI - Perhaps applying the new process to our CDN bundle builds - Generalized cleanup These will all be included in separate PRs, some in the immediate future and some in the hopefully-not-too-distant short-to-medium term.
AbhiPrasad
pushed a commit
that referenced
this pull request
May 30, 2022
This switches our `build` and `build:dev` yarn scripts to use the new rollup/sucrase build process. This is the culmination of a number of previous changes, whose highlights include: - #4724, which made building types files a separate build process - #4895, which updated the SDK to use TS 3.8.3 - #4926, which removed use of the `Severity` enum - #5005, which switch our default tsconfig to target es6 - #4992, which added the Sucrase plugin, some helper functions, and the `yarn build:rollup` script - #4993, which added rollup plugins to use `var` rather than `const` and clean up the built code in various ways - #5022, which applied the same `const`-to-`var` translation to tests - #5023, which added the ability to change injected polyfills into imports The result is that, as of this PR, we will no longer use `tsc` to transpile or down-complile our code when building npm packages. Instead, we will be using Rollup to handle making our code CJS-friendlly and Sucrase to handle the transpilation from TS to JS. The main advantages of this change are: - It forced us to do a lot of updating, centralizing, and cleanup of our tooling, not just for build but also for testing and linting. - It brings our CDN build process and our npm build process much more in line with each other, for easier maintainability. - It gives us more control over the eventual output, because we have access to a whole internet full of Rollup plugins (not to mention the ability to make our own), rather than being constrained to tsconfig options. (Plugins also allow us to interact with the code directly.) - It speeds up our builds fairly significantly. I ran a number of trials in GHA of running `yarn build:dev` at the top level of the repo. Before this change, the average time was ~150 seconds. After this change, it's about half that, roughly 75 seconds. Because of the switch in tooling, the code we publish is going to be slightly different. In order to make sure that those differences weren't going to be breaking, I built each package under the old system and under the new system, ran a `git diff`, and checked every file, both CJS and ESM, in every package affected by this change. The differences (none of which affect behavior or eventual bundle size by more than a few bytes in each direction), fell into a few categories: - Purely cosmetic changes, things like which comments are retained, the order of imports, where in the file exports live, etc. - Changes to class constructors, things like not explicitly assigning `undefined` to undefined attributes, using regular assignment rather than `Object.defineProperty` for attributes which are assigned values, and splitting some of those assignments off into helper functions. - Changes related to the upgrade to ES6 and dropping of support for Node 6, things like not polyfilling object spread or async/await While this represents the most significant part of the overall change, a few outstanding tasks remain: - Making this same switch in `build:watch` - Parallelizing the builds, both locally and in CI - Perhaps applying the new process to our CDN bundle builds - Generalized cleanup These will all be included in separate PRs, some in the immediate future and some in the hopefully-not-too-distant short-to-medium term.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As part of the new bundling process, the splits the building of types off into its own build step. Key changes:
A new, types-specific tsconfig in each package, which generates types only, and a todo in the main tsconfig.json for turning off the current type generation in
dist/andesm/. (It's not turned off now in order to not break anyone importing directly from those folders instead of from@sentry/xxxx. We'll flip the switch come v7, but for now we're just generating both.) This types tsconfig is also added to the templates.New yarn scripts for generating types (in regular and watch versions), and the addition of this script to other build commands.
A node script for running the repo-wide
yarn:build:typescommand, which solves the problem of the watch build hanging if a regular build hasn't happened first. As explained at the top of the script:A change to the
typesentry in all package.json files, to point to the types generated using the new tsconfig and the new scripts.Ref: https://getsentry.atlassian.net/browse/WEB-668