Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(ci): always bootstrap packages #26349

Merged
merged 4 commits into from
Nov 23, 2022
Merged

chore(ci): always bootstrap packages #26349

merged 4 commits into from
Nov 23, 2022

Conversation

liamdebeasi
Copy link
Contributor

@liamdebeasi liamdebeasi commented Nov 22, 2022

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
    • Some docs updates need to be made in the ionic-docs repo, in a separate PR. See the contributing guide for details.
  • Build (npm run build) was run locally and any changes were pushed
  • Lint (npm run lint) has passed locally and any fixes were made for failures

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Dependency Packages Not Symlinked

Using --scope causes other dependency packages to not be symlinked. For example, using --scope @ionic/angular will cause @ionic/core to not be symlinked in the angular package. As a result, framework packages do not necessarily build with the latest API information available to them.

This does not impact the contents of releases, but it does impact the ability to release a package if it tries to get a newer API from an older version of Ionic Core. I discovered this while trying to release a dev build of Ionic 7 where Angular tried to use a new interface that was not available in older versions of Ionic: https://github.com/ionic-team/ionic-framework/actions/runs/3527294227

Dependency Package Versions Not Changed

lerna version made sure that versions for dependencies were also bumped. So if @ionic/angular was bumped to 7.0.0-foo then its dependency of @ionic/core was also bumped to 7.0.0-foo. npm version does not do that for us.

Additionally, dependency packages use ^ which means that developers on older versions of Ionic could potentially get newer dependency versions.

Example

@ionic/angular: 6.3.8

In @ionic/angular's package.json, it has a dependency of @ionic/core: ^6.3.8

When Ionic Angular and Ionic Core 6.3.9 come out, if developers still on @ionic/[email protected] do a fresh install of their node modules, they will receive @ionic/[email protected] but with @ionic/[email protected].

This means that installing dev builds for a framework did not also install the dev build for @ionic/core.

What is the new behavior?

  • All packages are now symlinked by adding --include-dependencies to lerna bootstrap
  • Other framework packages also restore the core build as they pull from @ionic/core.
  • Removed npm version in favor of lerna version so dependency versions are updated.
  • The dependency versions are now updated exactly instead of using ^

Does this introduce a breaking change?

  • Yes
  • No

Other information

@liamdebeasi liamdebeasi mentioned this pull request Nov 23, 2022
7 tasks
@liamdebeasi liamdebeasi marked this pull request as ready for review November 23, 2022 14:25
@liamdebeasi liamdebeasi requested a review from a team as a code owner November 23, 2022 14:25
@liamdebeasi liamdebeasi merged commit 4b501fd into main Nov 23, 2022
@liamdebeasi liamdebeasi deleted the ci-bootstrap branch November 23, 2022 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant