fix(shared): Remove testUtils & smaller updates#1868
Conversation
🦋 Changeset detectedLatest commit: b5955bb The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
These were just all re-exports that did nothing
| @@ -1,11 +1 @@ | |||
| import '@testing-library/jest-dom'; | |||
There was a problem hiding this comment.
We had @testing-library/jest-dom/extend-expect in our testUtils file. This line already covers this
| @@ -0,0 +1,9 @@ | |||
| --- | |||
| "@clerk/shared": patch | |||
There was a problem hiding this comment.
🔧 I think that changing the public API (eg removing an export in our case) of a package requires a major version bump.
There was a problem hiding this comment.
It's an internal package without a README only used by our Jest tests that we also don't tell people to install or use. It'd consider it purely internal.
So while technically you're correct, I wouldn't care about that :)
nikosdouvlis
left a comment
There was a problem hiding this comment.
Awesome job @LekoArts 🔥
Remove the package.json we put into dist/cjs and dist/esm. They have no benefit since the root package.json already has sideEffects defined.
For future reference, old Vite React apps weren't picking up the root package.json's sideEffects prop. It seems that this has been fixed since then so all changes in this PR as spot on.
|
This PR has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
While working on JS-509 I quickly realized that putting that into one big PR would be a mess. So this is a slice of work that can be shipped separately and removes some noise from the original purpose of that ticket.
This PR does a couple of things:
testUtilsfrom@clerk/shared. There's no need to put this into such a package, it can live inside the repo outside of a npm package. The contents of thetestUtilswere re-exports from@testing-library/reactand an unused wrapper. So I didn't have to move it somewhere else, I just had to update the imports@clerk/shared. Through an incorrecttestRegexone test was never run. Jest's defaults are fine for our use case, so we don't even need to set itpackage.jsonwe put intodist/cjsanddist/esm. They have no benefit since the rootpackage.jsonalready hassideEffectsdefined.peerDependenciesMetato makereactpeerDep optionalLICENSEauthorChecklist
npm testruns as expected.npm run buildruns as expected.Type of change
Packages affected
@clerk/clerk-js@clerk/clerk-react@clerk/nextjs@clerk/remix@clerk/types@clerk/themes@clerk/localizations@clerk/clerk-expo@clerk/backend@clerk/clerk-sdk-node@clerk/shared@clerk/fastify@clerk/chrome-extensiongatsby-plugin-clerkbuild/tooling/chore