Skip to content

Conversation

@wobsoriano
Copy link
Member

@wobsoriano wobsoriano commented Jul 31, 2024

Description

See previous PR here.

This PR aims to migrate ClerkJS script loader functions to the shared package so it can be reused across official and community SDKs.

New functions exported in @clerk/shared:

  • loadClerkJsScript - Hotloads the Clerk JS script.
  • deriveState - Renders initial auth state from SSR if available
  • versionSelector

Closes ECO-4

Checklist

  • npm test runs as expected.
  • npm run build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

@changeset-bot
Copy link

changeset-bot bot commented Jul 31, 2024

🦋 Changeset detected

Latest commit: 090f0e7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 15 packages
Name Type
@clerk/astro Patch
@clerk/shared Patch
@clerk/clerk-react Patch
@clerk/nextjs Patch
@clerk/backend Patch
@clerk/chrome-extension Patch
@clerk/clerk-js Patch
@clerk/elements Patch
@clerk/clerk-expo Patch
@clerk/express Patch
@clerk/fastify Patch
@clerk/remix Patch
@clerk/clerk-sdk-node Patch
@clerk/tanstack-start Patch
@clerk/testing Patch

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

scriptHost = parsePublishableKey(publishableKey)?.frontendApi || '';
}

const variant = clerkJSVariant ? `${clerkJSVariant.replace(/\.+$/, '')}.` : '';

Check failure

Code scanning / CodeQL

Polynomial regular expression used on uncontrolled data

This [regular expression](1) that depends on [library input](2) may run slow on strings with many repetitions of '.'. This [regular expression](1) that depends on [library input](3) may run slow on strings with many repetitions of '.'.
@wobsoriano wobsoriano force-pushed the rob/eco-4-move-or-use-utilities-from-clerkshared branch from d0eebdd to 2ef2052 Compare August 1, 2024 00:03
@wobsoriano wobsoriano marked this pull request as ready for review August 1, 2024 00:15
Copy link
Contributor

@panteliselef panteliselef left a comment

Choose a reason for hiding this comment

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

If this is not urgent, I can review early next week 😊

@wobsoriano
Copy link
Member Author

If this is not urgent, I can review early next week 😊

Definitely not urgent, and thanks!

@wobsoriano
Copy link
Member Author

wobsoriano commented Aug 5, 2024

I believe we can drop the usage of PACKAGE_VERSION from versionSelector of clerk-react because for that package PACKAGE_VERSION and JS_PACKAGE_VERSION majors always match

@panteliselef that'd be awesome if that's the case. So we can just set the default packageVersion to JS_PACKAGE_VERSION and completely remove __internal_packageVersion. Just wondering if there any drawbacks in doing this. Same question I asked in the previous PR.

cc @nikosdouvlis

@wobsoriano wobsoriano changed the title chore(astro,shared): Move functions that can be reused across JavaScript SDKs chore(astro,clerk-react,shared): Move functions that can be reused across JavaScript SDKs Aug 5, 2024
@wobsoriano wobsoriano changed the title chore(astro,clerk-react,shared): Move functions that can be reused across JavaScript SDKs chore(astro,clerk-react,shared): Migrate functions that can be reused across JavaScript SDKs Aug 5, 2024
Copy link
Contributor

@LekoArts LekoArts left a comment

Choose a reason for hiding this comment

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

Looks good! Some smaller requests

@wobsoriano wobsoriano force-pushed the rob/eco-4-move-or-use-utilities-from-clerkshared branch from a313653 to 2fab204 Compare August 7, 2024 15:27
Comment on lines 14 to 24
const errorThrower = buildErrorThrower({ packageName: '@clerk/shared' });

/**
* Sets the package name for error messages during ClerkJS script loading.
*
* @example
* setClerkJsLoadingErrorPackage('@clerk/clerk-react');
*/
export function setClerkJsLoadingErrorPackageName(packageName: string) {
errorThrower.setPackageName({ packageName });
}
Copy link
Member Author

@wobsoriano wobsoriano Aug 7, 2024

Choose a reason for hiding this comment

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

This one is only used inside loadClerkJsScript. I named it like that and not export another setErrorThrowerOptions to avoid confusion. We might also need to add this to frontend SDK spec @LekoArts

I also thought of sharing this errorThrower across SDKs but that would mess with existing error throwers in next and react.

@wobsoriano wobsoriano changed the title chore(astro,clerk-react,shared): Migrate functions that can be reused across JavaScript SDKs chore(astro,clerk-react,shared,nextjs): Migrate functions that can be reused across JavaScript SDKs Aug 7, 2024
@wobsoriano wobsoriano force-pushed the rob/eco-4-move-or-use-utilities-from-clerkshared branch from b09d148 to 33c28b6 Compare August 7, 2024 17:20
@wobsoriano wobsoriano requested a review from nikosdouvlis August 8, 2024 13:43
@wobsoriano wobsoriano mentioned this pull request Aug 8, 2024
9 tasks
@wobsoriano
Copy link
Member Author

!snapshot

@clerk-cookie
Copy link
Collaborator

Hey @wobsoriano - the snapshot version command generated the following package versions:

Package Version
@clerk/astro 1.0.11-snapshot.v090f0e7
@clerk/backend 1.6.2-snapshot.v090f0e7
@clerk/chrome-extension 1.1.13-snapshot.v090f0e7
@clerk/clerk-js 5.14.0-snapshot.v090f0e7
@clerk/elements 0.12.4-snapshot.v090f0e7
@clerk/clerk-expo 2.1.0-snapshot.v090f0e7
@clerk/express 0.0.27-snapshot.v090f0e7
@clerk/fastify 1.0.29-snapshot.v090f0e7
@clerk/localizations 2.5.7-snapshot.v090f0e7
@clerk/nextjs 5.3.0-snapshot.v090f0e7
@clerk/clerk-react 5.4.0-snapshot.v090f0e7
@clerk/remix 4.2.13-snapshot.v090f0e7
@clerk/clerk-sdk-node 5.0.26-snapshot.v090f0e7
@clerk/shared 2.5.0-snapshot.v090f0e7
@clerk/tanstack-start 0.2.0-snapshot.v090f0e7
@clerk/testing 1.2.9-snapshot.v090f0e7
@clerk/themes 2.1.19-snapshot.v090f0e7
@clerk/types 4.13.0-snapshot.v090f0e7

Tip: Use the snippet copy button below to quickly install the required packages.
@clerk/astro

npm i @clerk/[email protected] --save-exact

@clerk/backend

npm i @clerk/[email protected] --save-exact

@clerk/chrome-extension

npm i @clerk/[email protected] --save-exact

@clerk/clerk-js

npm i @clerk/[email protected] --save-exact

@clerk/elements

npm i @clerk/[email protected] --save-exact

@clerk/clerk-expo

npm i @clerk/[email protected] --save-exact

@clerk/express

npm i @clerk/[email protected] --save-exact

@clerk/fastify

npm i @clerk/[email protected] --save-exact

@clerk/localizations

npm i @clerk/[email protected] --save-exact

@clerk/nextjs

npm i @clerk/[email protected] --save-exact

@clerk/clerk-react

npm i @clerk/[email protected] --save-exact

@clerk/remix

npm i @clerk/[email protected] --save-exact

@clerk/clerk-sdk-node

npm i @clerk/[email protected] --save-exact

@clerk/shared

npm i @clerk/[email protected] --save-exact

@clerk/tanstack-start

npm i @clerk/[email protected] --save-exact

@clerk/testing

npm i @clerk/[email protected] --save-exact

@clerk/themes

npm i @clerk/[email protected] --save-exact

@clerk/types

npm i @clerk/[email protected] --save-exact

@wobsoriano wobsoriano merged commit 7e0ced3 into main Aug 9, 2024
@wobsoriano wobsoriano deleted the rob/eco-4-move-or-use-utilities-from-clerkshared branch August 9, 2024 14:14
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.

5 participants