Skip to content

Conversation

jclem
Copy link
Contributor

@jclem jclem commented Feb 4, 2022

Since the "history" package is used by UnderlineNav, it should be in @primer/react's direct production dependencies. This ensures that its types are properly exposed to consumers of @primer/react that do not have skipLibCheck enabled.

Since this doesn't actually change anything in @primer/react at runtime or even in development, I don't believe this is worth a changelog.

@jclem jclem requested review from a team and mperrotti February 4, 2022 19:23
@changeset-bot
Copy link

changeset-bot bot commented Feb 4, 2022

⚠️ No Changeset found

Latest commit: b847180

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

@github-actions
Copy link
Contributor

github-actions bot commented Feb 4, 2022

size-limit report 📦

Path Size
dist/browser.esm.js 61.64 KB (0%)
dist/browser.umd.js 62 KB (0%)

Copy link
Contributor

@colebemis colebemis left a comment

Choose a reason for hiding this comment

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

Thanks, @jclem!

We should probably eventually update UnderlineNav to remove its dependency on history but this seems like a reasonable fix for now 👍

@colebemis colebemis added the skip changeset This change does not need a changelog label Feb 4, 2022
@jclem jclem merged commit 9567998 into main Feb 4, 2022
@jclem jclem deleted the add-history-dependency branch February 4, 2022 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip changeset This change does not need a changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants