-
Notifications
You must be signed in to change notification settings - Fork 261
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
fix(shared): Remove "sideEffects" and fix postbuild script #1974
Conversation
🦋 Changeset detectedLatest commit: 26d72e7 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 |
!snapshot |
This comment was marked as outdated.
This comment was marked as outdated.
"browser", | ||
"callWithRetry", | ||
"color", | ||
"cookie", | ||
"date", | ||
"deprecated", | ||
"error", | ||
"file", | ||
"globs", | ||
"handleValueOrFn", | ||
"isomorphicAtob", | ||
"keys", | ||
"loadScript", | ||
"localStorageBroadcastChannel", | ||
"poller", | ||
"proxy", | ||
"underscore", | ||
"url", | ||
"react" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ @LekoArts why are these added at top level ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@panteliselef Please see #1950
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ I was wondering the same thing as @panteliselef. Are we able to nest them?
We'll need to define these additional directories in two places and a nested approach will make it more manageable.
- NPM Script:
clean
will need to account for these generated files. - Turborepo: The generated files/dirs will also need to be listed in the
build
taskoutputs
.
As-is, they both only account for ./dist
.
Thanks!
This is a follow-up to #1950 so the functionality is not new and Turborepo does not need to be considered here as these are only package.json files for compat reasons |
We're not able to nest them as we've found that Expo does not respect the |
@@ -2,9 +2,27 @@ | |||
"name": "@clerk/shared", | |||
"version": "1.0.0", | |||
"description": "Internal package utils used by the Clerk SDKs", | |||
"sideEffects": false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you notice any side-effects (pun intended) in regards to code pruning when removing this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still all worked fine during bundling :)
…1983) * fix(clerk-expo,shared): Bundle shared in expo * Revert "fix(clerk-expo,shared): Bundle shared in expo" This reverts commit c1e0279. * fix(shared): Add files key and fix script * chore(repo): Linting * fix(shared): Add package.json to exports (cherry picked from commit a68eb30) Co-authored-by: Lennart <lekoarts@gmail.com>
Description
In the first commit I tried to bundle
@clerk/shared
into Expo but that didn't completely work as we'd also need to bundle React or Clerk JS -- ugh :/So this PR adds the
files
and adjusts the subpaths script.I also removed the
sideEffects: false
since tsup/esbuild complained that this package is not side-effects free. Which is true since some packages import shared chunks.This is a follow-up to #1950
Fixes SDK-884
Checklist
npm test
runs as expected.npm run build
runs as expected.Type of change
Packages affected
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/clerk-expo
@clerk/fastify
gatsby-plugin-clerk
@clerk/localizations
@clerk/nextjs
@clerk/clerk-react
@clerk/remix
@clerk/clerk-sdk-node
@clerk/shared
@clerk/themes
@clerk/types
build/tooling/chore