clerk / javascript

Official Javascript repository for Clerk authentication
https://clerk.com
MIT License
1.04k stars 235 forks source link

[SDK-815] Import of deprecated from @clerkjs/shared breaks Vite builds using @clerkjs/clerk-sdk-node #1883

Closed markjaquith closed 10 months ago

markjaquith commented 10 months ago

Preliminary Checks

Reproduction / Replay Link

https://github.com/markjaquith/clerk-sdk-node-4-12-13-bug

Publishable key

pk_test_Y2xvc2luZy1seW54LTkzLmNsZXJrLmFjY291bnRzLmRldiQ

Description

@clerk/clerk-sdk-node v4.12.13 introduced an import from @clerk/shared to handle deprecation notices.

This import breaks Vite builds, because of some CommonJS import nonsense.

With v4.12.12, Vite builds clean.

With v4.12.13 when running npm run build, I get:

file:///Users/mark/Sites/clerk-sdk-bug/node_modules/@clerk/clerk-sdk-node/dist/esm/chunk-IBBCRHOG.mjs:31
import { deprecated } from "@clerk/shared";
         ^^^^^^^^^^
SyntaxError: Named export 'deprecated' not found. The requested module '@clerk/shared' is a CommonJS module, which may not support all module.exports as named exports.

The fact that I'm using SvelteKit here isn't material. None of the functionality matters, just that Vite is now (in a patch update) choking on @clerk/shared apparently being a CommonJS module.

Reproduction

  1. Clone the above repo.
  2. npm i
  3. npm run build (see error)
  4. npm i @clerk/clerk-sdk-node@4.12.12 (revert to earlier version)
  5. npm run build (no error)

Environment

System:
    OS: macOS 14.0
    CPU: (10) arm64 Apple M1 Max
    Memory: 7.70 GB / 64.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.17.1 - ~/.nvm/versions/node/v18.17.1/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 9.6.7 - ~/.nvm/versions/node/v18.17.1/bin/npm
    pnpm: 7.21.0 - /opt/homebrew/bin/pnpm
  npmPackages:
    @clerk/clerk-js: ^4.61.0 => 4.61.0 
    @clerk/clerk-sdk-node: ^4.12.13 => 4.12.13 
    @clerk/types: ^3.55.0 => 3.55.0 
    @sveltejs/adapter-auto: ^2.1.0 => 2.1.0 
    @sveltejs/kit: ^1.5.0 => 1.25.2 
    @typescript-eslint/eslint-plugin: ^5.45.0 => 5.62.0 
    @typescript-eslint/parser: ^5.45.0 => 5.62.0 
    autoprefixer: ^10.4.14 => 10.4.16 
    dotenv: ^16.3.1 => 16.3.1 
    eslint: ^8.28.0 => 8.51.0 
    eslint-config-prettier: ^8.5.0 => 8.10.0 
    eslint-plugin-svelte: ^2.26.0 => 2.34.0 
    postcss: ^8.4.23 => 8.4.31 
    prettier: ^2.8.0 => 2.8.8 
    prettier-plugin-svelte: ^2.8.1 => 2.10.1 
    react: ^18.2.0 => 18.2.0 
    svelte: ^3.54.0 => 3.59.2 
    svelte-check: ^3.0.1 => 3.5.2 
    tailwindcss: ^3.3.2 => 3.3.3 
    tslib: ^2.4.1 => 2.6.2 
    typescript: ^5.2.2 => 5.2.2 
    vite: ^4.4.11 => 4.4.11

SDK-815

dimkl commented 10 months ago

Hello @markjaquith and thank you for reporting this one. We are currently making some changes to our bundling of @clerk/shared package. We will check this and let you know.

markjaquith commented 10 months ago

I'm thinking that @clerk/clerk-sdk-node shouldn't have a dependency on @clerk/shared as it stands.

@clerk/shared depends on react, so now even non-React projects that are using the node SDK to do server-side auth have to depend on React.

See this report in Discord: https://discord.com/channels/856971667393609759/1163476965563572234

LekoArts commented 10 months ago

@clerk/shared has a peerDependency on react as it stands, so it doesn't automatically pulls it in. But you can run into issues with it, yes. In https://github.com/clerkinc/javascript/pull/1868 I made that optional through https://docs.npmjs.com/cli/v7/configuring-npm/package-json#peerdependenciesmeta

But it still has to be released through https://github.com/clerkinc/javascript/pull/1867. There's also more work in progress to isolate the parts through npm subpaths so that in e.g. Node environment the React parts never get pulled in.

I'll let you know once we have a version to test.

LekoArts commented 10 months ago

Hey 👋

Can you please try this version:

npm install @clerk/clerk-sdk-node@4.12.15-snapshot.v552195d --save-exact

I tried it with your reproduction and the build works. Please see if in addition to the build succeeding also the functionality works 😆

P.S.: Your reproduction was missing an .env file so at first my build failed on missing env vars

markjaquith commented 10 months ago

I tried @clerk/clerk-sdk-node@4.12.15-snapshot.v552195d on the full app that this example was boiled down from. No build issues, and Clerk logins are working!

LekoArts commented 10 months ago

@markjaquith Awesome! I pushed a change, could you also try this version?

npm i @clerk/clerk-sdk-node@4.12.15-snapshot.vf9232b3 --save-exact

Thanks!

markjaquith commented 10 months ago

That version is also building and working for me. 👍🏻