fix(clerk-react): Fix build process not catching type errors [SDK-1065]

[tmilewski]

Description

tsup doesn't reliably handle type-checking, thus we must continue to rely on tsc.

SDK-1065

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:

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

[changeset-bot]

🦋 Changeset detected

Latest commit: 0cdc277

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

This PR includes changesets to release 13 packages

Name	Type
@clerk/clerk-react	Patch
@clerk/clerk-sdk-node	Patch
@clerk/localizations	Patch
@clerk/shared	Patch
@clerk/types	Patch
@clerk/chrome-extension	Patch
@clerk/clerk-expo	Patch
gatsby-plugin-clerk	Patch
@clerk/nextjs	Patch
@clerk/remix	Patch
@clerk/clerk-js	Patch
@clerk/backend	Patch
@clerk/fastify	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

[dimkl]

@tmilewski We should probably do this for the rest of the packages:

• expo
• remix
• nextjs
• types
• shared
• backend
  ....
  Could you take a look before proceeding with only this one? Also i would suggest we use tsc on Success and put emitDeclarationsOnly: true in tsconfig.json.
  Also there is a run helper runAfterLast from scripts/utils.

[github-actions]

⚠️ Changes detected under the ClerkJS ui directory!

Don't forget to apply the same changes under the /ui.retheme directory:
packages/clerk-js/src/ui/** ➡️ packages/clerk-js/src/ui.retheme/**

Also, you may need to update the following files:

• packages/localizations/src/en-US.retheme.ts
• packages/localizations/src/index.retheme.ts
• packages/types/src/appearance.retheme.ts
• packages/types/src/clerk.retheme.ts
• packages/types/src/index.retheme.ts
• packages/types/src/localization.retheme.ts

[dimkl]

🔧 I think that those keys are dropped in Retheme project as part of #2290
Is there an error somewhere that indicated to make these changes?

[tmilewski]

Yes, it was giving me errors when running tsc. If they're supposed to be dropped then I can look into it more.

[dimkl]

❓ isn't this change added in #2314 ?

[tmilewski]

It is, it was the only way I could get this to build and test, as it wasn't available at the time. My plan was to rebase everything and weed it out once it came time.

[dimkl]

🔧 Those keys are only added in Retheme project and they are supposed to exist in packages/types/src/localization.retheme.ts and not here. We should remove them from this file.

[LekoArts]

Why not set dts: false then? Then we'd do double the work

[nikosdouvlis]

Folks, are we sure this is the case? Are there any open issues I can read for more context?

I tested tsup locally with a few isolated scenarios and the build fails as expected.

In any case, let's wait before we consider merging this as we are making too many build-related changes across the codebase for v5.

[tmilewski]

@nikosdouvlis I was able to get it to consistently succeed/fail in various scenarios. That said, I agree that we should hold, and I'm going to close for the time being.