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(scripts): Use relaxed peer dependencies to avoid triggering major version bumps #4736

Merged
merged 6 commits into from
Dec 13, 2022

Conversation

chaance
Copy link
Collaborator

@chaance chaance commented Dec 1, 2022

Consider this scenario:

  1. Run yarn changeset:version to bump versions
  2. Changesets sees that a package has a minor change
  3. Because we release packages in lockstep, all packages get a minor update
  4. @remix-run/dev has "peerDependencies": { "@remix-run/serve": "1.8.0" }
  5. This dependency will now be out of range after the update
  6. Changesets makes the safe bet and updates @remix-run/dev to 2.0.0 because it can't be sure this doesn't result in a breaking change
  7. Because we release packages in lockstep, all packages get a major update

In practice, this means any minor changeset will result in a major bump, which definitely isn't what we want. This means I end up manually updating all of the package versions which ... kinda sucks.

Instead, we relaxe the peer dependency range for internal packages. That way the update doesn't result in an out-of-range peer dependency, and all packages are bumped to the next minor release instead.

Because changesets doesn't automatically bump peer dependencies with the relaxed range (which makes sense in most cases), I added a script does that for us. This makes the change safer because updating the leading dependency will result in a peer dependency warning if the user doesn't bump the peer dependency for some reason.

Thanks to @Andarist for the guidance here and the original script!

@changeset-bot
Copy link

changeset-bot bot commented Dec 1, 2022

⚠️ No Changeset found

Latest commit: 908f826

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@Andarist
Copy link
Contributor

Andarist commented Dec 1, 2022

Because changesets doesn't automatically bump peer dependencies with the relaxed range (which makes sense in most cases)

I wouldn't say that this makes sense in most cases though. It's a contextual decision. For example, it's not a good default for Babel plugin (usually they depend on ^7) or UI kit libraries with a peer dep on a framework (when depending on React you might want to depend on a version with a certain feature, like if you depend on hooks then u stay on the ^16.8 range).

I think though that this strategy makes sense in your case.

@chaance
Copy link
Collaborator Author

chaance commented Dec 1, 2022

I wouldn't say that this makes sense in most cases though. It's a contextual decision. For example, it's not a good default for Babel plugin (usually they depend on ^7) or UI kit libraries with a peer dep on a framework (when depending on React you might want to depend on a version with a certain feature, like if you depend on hooks then u stay on the ^16.8 range).

Updated the inline comment as not to sully your good name 😀

@chaance chaance merged commit c4957c1 into dev Dec 13, 2022
@chaance chaance deleted the chance/changesets-peer-deps-fix branch December 13, 2022 18:18
@github-actions github-actions bot added the awaiting release This issue has been fixed and will be released soon label Dec 13, 2022
pcattori added a commit that referenced this pull request Dec 16, 2022
* fix(remix-dev): convert `config.appDirectory` to relative unix path (#4709)

* fix(remix-dev): convert appDirectory to unix style for fast-glob

Signed-off-by: Logan McAnsh <[email protected]>

* chore: relative path

Signed-off-by: Logan McAnsh <[email protected]>

* chore: add test

Signed-off-by: Logan McAnsh <[email protected]>

* fix test

Signed-off-by: Logan McAnsh <[email protected]>

* fix: typo

Signed-off-by: Logan McAnsh <[email protected]>

* chore: update test

Signed-off-by: Logan McAnsh <[email protected]>

Signed-off-by: Logan McAnsh <[email protected]>

* chore: add changeset for #4709 (#4718)

* ci: add typechecking for deno (#4715)

* ci: add typechecking for deno

* ci: install deno for integration tests

* chore: format

* chore: format

* fix: Firefox LiveReload (#4725)

Firefox infinitely reloads the page as long as `<LiveReload>` is rendering.

Closes #4692

* fix(remix-dev): allow defining multiple routes for the same route module file (#3970)

* Allow multiple routes for same route module

* Update packages/remix-dev/config/routes.ts

Co-authored-by: Andrew Leedham <[email protected]>

* Update routes.ts

- Better name for automated ID variable;
- Small adjust in `id` option comment;

* - Removing redundant IF

* Update routes.ts

Revert complex custom ID in routes

* Non unique custom routes ID error and test

* Update assets.ts

Trying to solve a conflict

* Revert "Update assets.ts"

This reverts commit 2064c57.

* Error on collisions with non-custom routeIds

* Create big-spoons-grab.md

Co-authored-by: Andrew Leedham <[email protected]>
Co-authored-by: Matt Brophy <[email protected]>

* feat: Allow pass-through script props in `ScrollRestoration` (#2879)

* ci(nightly): add deno for typechecking deno files (#4738)

* ci: fix race condition writing globals.d.ts shim (#4717)

Co-authored-by: Chance Strickland <[email protected]>

* chore: bump @playwright/test to latest (#4749)

Signed-off-by: Logan McAnsh <[email protected]>

Signed-off-by: Logan McAnsh <[email protected]>

* Fix 4199: TypedResponse allows incompatible types (#4734)

* Fixes #4199: Do not allow assignment of incompatible TypedResponses

* Add myself to contributors.yml

* Create light-sheep-give.md

* slight changeset tweak

* additional changeset tweaks

Co-authored-by: Pedro Cattori <[email protected]>

* chore: format

* test: add transition integration tests (#4739)

test: useTransition to wait for states

This approach could probably be applied across other flakey tests and could also be documented as a good approach of if there is user-feedback for a specific action when running integration tests.

* feat: testing helpers (#4539)

Co-authored-by: James Restall <[email protected]>
Signed-off-by: Logan McAnsh <[email protected]>

* chore: format

* chore(remix-testing): update dependencies (#4756)

* fix(remix-testing): fix deps (#4757)

* Fix deps for new remix-testing package

* fix lint

* Remove ENABLE_REMIX_ROUTER flags (#4732)

* chore: add `@remix-run/testing` to changesets (#4781)

* chore: add `@remix-run/testing` to changesets

Signed-off-by: Logan McAnsh <[email protected]>

* chore: update `ADDING_A_PACKAGE.md`

Signed-off-by: Logan McAnsh <[email protected]>

Signed-off-by: Logan McAnsh <[email protected]>

* refactor(remix-react): upgrade Remix to `[email protected]` (non-data-router) and drop `history` (#4731)

* Bump remix to [email protected] (#4668)

Co-authored-by: Mehdi Achour <[email protected]>

* Bump remix to RR 6.4.4 and drop history dependency (#4702)

Co-authored-by: Mehdi Achour <[email protected]>

* ci(nightly): move git operations after build (#4797)

* ci(nightly): move git operations after build

when adding deno typechecking (#4715), nightly now refers to the version we just calculated which hasnt been published when trying to build and typecheck

Signed-off-by: Logan McAnsh <[email protected]>

* ci: update tmp branch name

Signed-off-by: Logan McAnsh <[email protected]>

Signed-off-by: Logan McAnsh <[email protected]>

* perf(remix-dev): Optimize `parentRouteId` lookup in `defineConventionalRoutes` (#4538)

* Use object for parentRouteId lookup

* Sign CLA

* Move parentRouteId logic to a separate function

* Update test fixture path

* chore: format

* chore(dev): add changeset for PR #4538 (#4800)

* chore: format

* refactor(remix-react): use `react-router-dom` import instead of `react-router` (#3325)

* chore: manually bump remix-testing version due to not being on main just yet

Signed-off-by: Logan McAnsh <[email protected]>

* fixup! chore: manually bump remix-testing version due to not being on main just yet

* fixup! chore: manually bump remix-testing version due to not being on main just yet

Signed-off-by: Logan McAnsh <[email protected]>

* chore: unify error usage (#4696)

* chore: format

* Add fetcher state/type tests (#4803)

* chore(deps): bump esbuild to latest (#4754)

* chore: add invariant instead of using `!`

Signed-off-by: Logan McAnsh <[email protected]>

* chore(deps): bump esbuild to latest

Signed-off-by: Logan McAnsh <[email protected]>

* Create fresh-shrimps-join.md

Signed-off-by: Logan McAnsh <[email protected]>
Co-authored-by: Pedro Cattori <[email protected]>

* test(integration): close server synchronously  (#4785)

* chore: normalize afterAll

Signed-off-by: Logan McAnsh <[email protected]>

* test: close server synchronously

Signed-off-by: Logan McAnsh <[email protected]>

* chore: appFixture.close isnt async anymore

Signed-off-by: Logan McAnsh <[email protected]>

* Update integration/helpers/create-fixture.ts

Co-authored-by: Pedro Cattori <[email protected]>

Signed-off-by: Logan McAnsh <[email protected]>
Co-authored-by: Pedro Cattori <[email protected]>

* feat: remix optional segments (#4706)

* feat: transform optional routes from remix to react router

* Add to contributors

* Add changeset

* fix(optional-segments): fix escaping of parenthesis

* small function fix

* Update packages/remix-dev/__tests__/routesConvention-test.ts

Co-authored-by: Pedro Cattori <[email protected]>

Co-authored-by: Pedro Cattori <[email protected]>

* chore: format

* chore: edit the optional segments changeset (#4815)

to make it clear that Remix won't support optional segments until integrated with React Router 6.5

* chore: format

* docs: rearrange docs, give everything its own page (#4821)

* chore: format

* fix: wrong parentheses in the optional segments changeset (#4825)

* fix: wrong parentheses in the optional segments changeset

* add: akamfoad to the contributers

* ci(nightly): add workflow_call for testing purposes

Signed-off-by: Logan McAnsh <[email protected]>

* Revert "ci(nightly): add workflow_call for testing purposes"

This reverts commit f3fa77e.

* chore(scripts): Use relaxed peer dependencies to avoid triggering major version bumps (#4736)

- Add script to bump peer deps with changesets version

Co-authored-by: Mateusz Burzyński <[email protected]>

* Add integration tests for request structures (#4829)

* fix(remix-dev): resolve asset entry full path to support mono-repo import of styles (#4855)

* chore: have eslint report unused eslint comments (#4863)

* chore: have eslint report unused eslint comments

Signed-off-by: Logan McAnsh <[email protected]>

* chore: remove additional comment

Signed-off-by: Logan McAnsh <[email protected]>

* perf(remix-architect,remix-netlify): improve performance of `isBinaryType` (#4761)

Co-authored-by: Logan McAnsh <[email protected]>
Co-authored-by: Pannatier Guillaume <[email protected]>

* chore: format

* chore(remix-testing): remove internal installGlobals (#4755)

* chore: remove internal installGlobals

Signed-off-by: Logan McAnsh <[email protected]>

* chore: add README

Signed-off-by: Logan McAnsh <[email protected]>

* Create quick-cats-fix.md

* Update .changeset/quick-cats-fix.md

Co-authored-by: Michaël De Boey <[email protected]>

* chore(deps): remove jsdom and happydom from devDependencies

Signed-off-by: Logan McAnsh <[email protected]>

Signed-off-by: Logan McAnsh <[email protected]>
Co-authored-by: Michaël De Boey <[email protected]>

* fix(dev): build js modules for ts->js conversion

The TS->JS migration was removed from the CLI codemod options, but still
used for TS->JS conversion when creating a new Remix project from the
CLI. The TS modules responsible for the TS->JS conversion were
incorrectly removed from the Rollup build, resulting in the
corresponding built JS modules being absent. That caused the CLI to
error when trying to perform TS->JS conversion. This changes
reintroduces the wiring to build the modules responsible for the TS->JS
conversion.

Fixes #4854

Signed-off-by: Logan McAnsh <[email protected]>
Co-authored-by: Logan McAnsh <[email protected]>
Co-authored-by: Matt Kane <[email protected]>
Co-authored-by: Remix Run Bot <[email protected]>
Co-authored-by: Chance Strickland <[email protected]>
Co-authored-by: Ryan Florence <[email protected]>
Co-authored-by: Lucas Ferreira <[email protected]>
Co-authored-by: Andrew Leedham <[email protected]>
Co-authored-by: Matt Brophy <[email protected]>
Co-authored-by: dabdine <[email protected]>
Co-authored-by: Jacob Ebey <[email protected]>
Co-authored-by: James Restall <[email protected]>
Co-authored-by: Michaël De Boey <[email protected]>
Co-authored-by: Mehdi Achour <[email protected]>
Co-authored-by: Dylan Markow <[email protected]>
Co-authored-by: Daniel Rios <[email protected]>
Co-authored-by: Akam Foad <[email protected]>
Co-authored-by: Mateusz Burzyński <[email protected]>
Co-authored-by: Guillaume Pannatier <[email protected]>
Co-authored-by: Pannatier Guillaume <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting release This issue has been fixed and will be released soon CLA Signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants