Skip to content
This repository was archived by the owner on Aug 30, 2023. It is now read-only.

ref(build): Align package.json build scripts with JS SDK scripts#320

Merged
Lms24 merged 4 commits intomainfrom
lms-pkgjson-scripts
Nov 21, 2022
Merged

ref(build): Align package.json build scripts with JS SDK scripts#320
Lms24 merged 4 commits intomainfrom
lms-pkgjson-scripts

Conversation

@Lms24
Copy link
Copy Markdown
Member

@Lms24 Lms24 commented Nov 18, 2022

This PR aligns the scripts in package.json with the scripts we use in the JS SDK repo. Doing this now doesn't hurt us and it's one less thing to worry about when migrating. Admittedly, the build:* scripts look a little scary now but this is just because I wanted to preserve the separate build scripts for worker and core. Therefore, we'll end up with a few more scripts than we have in most other SDKs (s.g. Svelte).

In the monorepo, we don't set env variables for prod vs. dev builds. I still need to think about what we're going to do about them but we can make these changes iteratively.

Here's a quick overview of the most important changes. All these scripts are used in the monorepo, not just by us but also by CI, so we need to align them:

  • build now serves as a "default" build command. I renamed what used to be the build script to build:rollup (which is also consistent with the monorepo). I left build:prod from devEx reasons for now but we can remove it after migrating.
  • build:dev basically remains unchanged as it's already mostly consistent with the monorepo
  • Added build:watch and build:dev:watch to re-build on file changes. I think previously, the --watch param was only applied to build-core but I figured we could also watch for worker changes. Which is why I added the build:all:watch sub-script to do this. I left the dev script for now to not disturb devEx but after migrating we can remove it, as it isn't used in the monorepo at all.
  • Added build:npm script to pack a tarball when publishing. In the monorepo, we use a script that makes some changes to package.json before packing. We'll need to add this here as well but not now, as it requires more changes. Also, I changed the "Build" GH workflow to use this command instead of npm pack.
  • Added lint and fix scripts that use eslint and prettier. I left the original commands eslint untouched, just changed the script names around it (and added the prettier stuff)

I added the npm-run-all dev dependency for the time being, to get the run-p/s commands we use in the monorepo. After migrating, we can remove the dependency again from this package because it's registered as a top-level dependency in the monorepo.

ref #311

@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 18, 2022

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
dist/index.js 34.88 KB (0%) 698 ms (0%) 72 ms (-32.81% 🔽) 769 ms

Copy link
Copy Markdown
Member

@billyvg billyvg left a comment

Choose a reason for hiding this comment

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

LGTM

@Lms24 Lms24 force-pushed the lms-pkgjson-scripts branch from f371bcf to 6a73681 Compare November 21, 2022 13:12
@Lms24 Lms24 merged commit 8d341fb into main Nov 21, 2022
@Lms24 Lms24 deleted the lms-pkgjson-scripts branch November 21, 2022 13:15
mydea pushed a commit to getsentry/sentry-javascript that referenced this pull request Nov 23, 2022
…etsentry/sentry-replay#320)

Align the scripts in `package.json` with the scripts we use in the JS SDK repo. Doing this now doesn't hurt us and it's one less thing to worry about when migrating. Admittedly, the `build:*` scripts look a little scary now but this is just because I wanted to preserve the separate build scripts for worker and core. Therefore, we'll end up with a few more scripts than we have in most other SDKs (s.g. [Svelte](https://github.com/getsentry/sentry-javascript/blob/d22380097b70bc503613cebbf2b914175174221c/packages/svelte/package.jsongetsentry/sentry-replay#L34-L42)).

In the monorepo, we don't set env variables for prod vs. dev builds. I still need to think about what we're going to do about them but we can make these changes iteratively. 

Here's a quick overview of the most important changes. All these scripts are used in the monorepo, not just by us but also by CI, so we need to align them:
* `build` now serves as a "default" build command. I renamed what used to be the `build` script to `build:rollup` (which is also consistent with the monorepo). I left `build:prod` from devEx reasons for now but we can remove it after migrating.
* `build:dev` basically remains unchanged as it's already mostly consistent with the monorepo 
* Added `build:watch` and `build:dev:watch` to re-build on file changes. I think previously, the `--watch` param was only applied to `build-core` but I figured we could also watch for worker changes. Which is why I added the `build:all:watch` sub-script to do this. I left the `dev` script for now to not disturb devEx but after migrating we can remove it, as it isn't used in the monorepo at all.
* Added `build:npm` script to pack a tarball when publishing. In the monorepo, we use a script that makes some changes to `package.json` before packing. We'll need to add this here as well but not now, as it requires more changes. Also, I changed the "Build" GH workflow to use this command instead of `npm pack`.
* Added `lint` and `fix` scripts that use eslint and prettier. I left the original commands eslint untouched, just changed the script names around it (and added the prettier stuff)

I added the `npm-run-all` dev dependency for the time being, to get the `run-p/s` commands we use in the monorepo. After migrating, we can remove the dependency again from this package because it's registered as a top-level dependency in the monorepo.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants