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

9.0.0 #190

Closed
wants to merge 5 commits into from
Closed

9.0.0 #190

wants to merge 5 commits into from

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Jun 4, 2024

This is the release candidate for version 9.0.0.

github-actions and others added 2 commits June 4, 2024 19:35
@MajorLift MajorLift self-assigned this Jun 4, 2024
@MajorLift MajorLift marked this pull request as ready for review June 4, 2024 19:51
@MajorLift MajorLift requested a review from a team as a code owner June 4, 2024 19:51
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Elliot Winkler <[email protected]>
CHANGELOG.md Outdated Show resolved Hide resolved
- Bump dependency `semver` from `^5.7.1` to `^7.6.0` ([#181](https://github.com/MetaMask/utils/pull/181)).

### Fixed
- **BREAKING:** Replace dependency `superstruct` `^1.0.3` with ESM-compatible `@metamask/superstruct` `^3.0.0` ([#185](https://github.com/MetaMask/utils/pull/185)).
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should note why this is breaking? Or is this breaking? I think we still use the same functionality.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here's the compare link for the package.json diff between these versions. The exports field and the dual type declaration files are added after we switch to @metamask/superstruct.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or wait would this dependency replacement not be a breaking change even if the dependency contains breaking changes?

Copy link
Contributor

@MajorLift MajorLift Jun 5, 2024

Choose a reason for hiding this comment

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

My intuition is that since superstruct@^1.0.3 broke downstream packages of @metamask/utils by preventing them from being imported into TypeScript projects (e.g. core/*) that use the NodeNext setting for moduleResolution, the fixes introduced by @metamask/superstruct should count as breaking changes.

By that logic, it seems the type declaration files fix might also be a breaking change, since it fixes an issue that was breaking downstream packages, not just directly but also via nested dependencies.

Both of these might count as cases where a fix may need to be announced as a breaking change.

Copy link
Contributor

@mcmire mcmire Jun 5, 2024

Choose a reason for hiding this comment

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

Hmm. superstruct is definitely unusable in a project that uses moduleResolution of nodenext, and that means that @metamask/utils is unusable too. So, upgrading this dependency makes @metamask/utils usable again (i.e. it's a fix). So, are there cases where this upgrade would break other kinds of TypeScript projects that currently use @metamask/utils? I don't believe the changes to the exports propagate to @metamask/utils, so people would be able to import it the same way. But, for instance, are there any specific TS compiler settings that a project would no longer be able to use after this upgrade?

Copy link
Contributor

Choose a reason for hiding this comment

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

All good points! I'm finding that reverting the TypeScript version and module, moduleResolution settings on the core v5.0 upgrade PR does cause breakage, but so far I'm mostly seeing errors related to the multiformats dynamic import. I'll try out other compiler options combinations to see if any errors related to @metamask/utils pop up.

Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't look like the switch to @metamask/superstruct causes any new restrictions for downstream packages. Since there are no breaking changes left, I'll close this PR and create a new minor release.

@MajorLift
Copy link
Contributor

Closing to convert to minor release.

@MajorLift MajorLift closed this Jun 6, 2024
@MajorLift MajorLift deleted the release/9.0.0 branch June 6, 2024 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants