[Bug]: build steps are run twice during CI

[johnhooks]

Is there an existing issue for this?

• I have searched the existing issues

Current behavior

During local development I've notice when running yarn install it will call the prepare scripts of all the workspace packages. Is this intended behavior?

This also happens in the CI build pipeline. So babel, tsc and api-extractor are all run twice. Because they are run after yarn install and also during yarn build. What is the right way to remove this duplication?

Certain script names are special. If defined, the preinstall script is called by yarn before your package is installed. For compatibility reasons, scripts called install, postinstall, prepublish, and prepare will all be called after your package has finished installing.

^^ reference

Expected behavior

Build steps should not be duplicated in CI or local development.

Steps to reproduce

yarn install
yarn build

Version

2.0.0-alpha.9

Environment

Node.js 16.13.0

Code of Conduct

• I agree to follow this project's Code of Conduct

[johnhooks]

The prepare script is bring rerun in every CircleCI job. So to complete the whole workflow it is run 6 times unnecessary.

[sarahdayan]

This was initially necessary to check the types and run tests.

It's no longer necessary during tests since 2d408ca, and no longer necessary during type checks since 676b1b6.

It's still necessary to have access to the built artifacts, though, when checking the build size and when building the documentation site (for this page to exist).

The CircleCI job that tests the build size seems to be the only one that requires a build now. In this case, we might do it directly in config.yml (and just the UMD builds) instead.

Regarding the release, Ship.js takes care of building anyway, so building on prepare isn't useful.

Finally, regarding the documentation, this is taken care of not in the CI but directly on Vercel, using their GitHub integration. We could handle this by building as the first step in all website: scripts. Bonus if we can build only if necessary so we don't re-build whenever we stop and start the server locally.

What do you think?

[johnhooks]

This will need to be reconsidered if prepare is changed

.codesandbox/ci.json

{
  "buildCommand": false,
  "^": "buildCommand is false because `yarn prepare` is going to build packages anyway.",
}

[johnhooks]

Take aways

• Removing prepare from all the package scripts would be okay.
  NOTE: I am going to add a prepare script to the currencies package to generate the currency files in PR refactor: generate currency source files #670. That would be necessary for many other steps of the workflow. (Though its a pretty quick step)

• Having a build job in the workflow is still necessary. Though none of the jobs ,other than test_size, require it.

• Most jobs don't need to attach the workspace, unless they need the build artifacts, like test_size

• Building the website and examples in CircleCI isn't necessary because they are part of the ci/codesandbox and integrations with Vercel.

[johnhooks]

Should the example projects have "private": true in their package.json?

They aren't published to npm and add it would disable building them in CircleCI because all the lerna commands have the --no-private flag applied.

[sarahdayan]

Should the example projects have "private": true in their package.json?

We can do this, yes!

[johnhooks]

@sarahdayan is the test_types job necessary? The build job runs tsc and api-extractor to generate the declaration files. If there is a type error the build job will fail.

[sarahdayan]

@johnhooks Hmm I guess it's useful to see at a glance, in the CI, what has failed. If the test_types job is red I know immediately what the issue is, so does the person who opened a PR but doesn't have access to CircleCI.

If this happens in the build job you don't immediately know what the problem is.

Is test_types a bottleneck right now?

[johnhooks]

It's just a duplication. If its helpful to you, then I'll shelve the idea. If I can figure out how to use Turbo effectively in CI, it will be a none issue because the build steps could be structured in a way to prevent a second call to tsc but still show you the error is a type issue.