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: upgrade dependencies #56

Merged
merged 18 commits into from
Apr 23, 2024
Merged

chore: upgrade dependencies #56

merged 18 commits into from
Apr 23, 2024

Conversation

pvditto
Copy link
Contributor

@pvditto pvditto commented Apr 18, 2024

  • Upgrades dependencies to latest compatible versions,
  • removes pinning of react and react-dom dev-dependencies (were pinned to v18.0.0),
  • updates @dittolive/ditto usage following update to latest version,
  • adds karma.conf.js to .eslintignore as the project is using a Typescript linter,
  • configures karma-typescript to handle ES2020 syntax used by cbor library (fixes Errors running tests using NPM #48),
  • updates targeted browsers to match https://docs.ditto.live/compatibility/js-web,
  • and applies all the updated linter / code style rules to the existing code.

Example project

  • Upgrades dependencies,
  • fixes test code,
  • and adds a TextEncoder polyfill required for JSDom to support @dittolive/ditto.

CI

  • Upgrades Github Actions versions and Node.js version used.

To be squash-merged.

@pvditto pvditto self-assigned this Apr 18, 2024
@pvditto pvditto requested a review from jreyes33 April 18, 2024 12:09
@pvditto pvditto marked this pull request as ready for review April 18, 2024 12:09
@pvditto pvditto linked an issue Apr 18, 2024 that may be closed by this pull request
Copy link
Member

@jreyes33 jreyes33 left a comment

Choose a reason for hiding this comment

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

Thanks for tackling this! LGTM, except for a small detail about how we generate changelogs.

CHANGELOG.md Outdated
Comment on lines 7 to 14
### Unreleased

## Changed

- browser support now matches the `@dittolive/ditto` package. see [Ditto's
documentation](https://docs.ditto.live/compatibility/js-web)
([#56](https://github.com/getditto/react-ditto/pull/56))

Copy link
Member

@jreyes33 jreyes33 Apr 18, 2024

Choose a reason for hiding this comment

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

Sorry for the lack of docs in this regard, but for the last few releases, we've been using conventional commits and standard-version to generate this changelog. I tried running it again (npx standard-version) to see what would happen with this manually added entry and it resulted in this:

### [0.11.1](https://github.com/getditto/react-ditto/compare/v0.11.0...v0.11.1) (2024-04-18)


### Bug Fixes

* remove implicit dependency on `lodash` ([9b56f93](https://github.com/getditto/react-ditto/commit/9b56f9331de4e5718478717d0c9527dca2764d15))

## [0.11.0](https://github.com/getditto/react-ditto/compare/v0.11.0-alpha.0...v0.11.0) (2023-03-22)

### Unreleased

## Changed

- browser support now matches the `@dittolive/ditto` package. see [Ditto's
  documentation](https://docs.ditto.live/compatibility/js-web)
  ([#56](https://github.com/getditto/react-ditto/pull/56))

Since this PR mainly focuses on upgrading dependencies, I wouldn't want to rewrite the whole PR's commit message when merging it just to see a new entry in the changelog. Is there a chance we could separate/reword this change to use a properly formatted commit message? Let me know if you have other ideas.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I just saw the changelog and wanted to add to it. I will remove that change and update the PR title to use the standard-version syntax.

Copy link
Contributor Author

@pvditto pvditto Apr 22, 2024

Choose a reason for hiding this comment

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

Hmm, adding the markdown link to the documentation in the PR title sure looks wonky. I can squash-merge the PR with a commit message

chore: match supported browsers with the `@dittolive/ditto` package. see [Ditto's documentation](https://docs.ditto.live/compatibility/js-web) for details.

does that work? It will hide the actual (internal) changes of this PR but at least it doesn't break the changelog tool.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think that works. I'd just like to point out that only commits marked with feat and fix will make it to the changelog, so if you want to have that message show there you should pick one of those; if not, chore seems fine to me too.

@pvditto pvditto changed the title Upgrade dependencies chore: upgrade dependencies Apr 22, 2024
@pvditto pvditto requested a review from jreyes33 April 22, 2024 10:25
Copy link
Member

@jreyes33 jreyes33 left a comment

Choose a reason for hiding this comment

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

LGTM!

@pvditto pvditto merged commit 35c29d4 into master Apr 23, 2024
1 check passed
@pvditto pvditto deleted the pv/update-dependencies branch April 23, 2024 10:37
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.

Update dependencies Errors running tests using NPM
2 participants