-
Couldn't load subscription status.
- Fork 402
fix(fastify): Disable minification for static import compatibility #7043
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
Conversation
🦋 Changeset detectedLatest commit: 8fbe205 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
WalkthroughPackage build and exports for @clerk/fastify were flattened: ESM/CJS output and type locations moved from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
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.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
packages/fastify/package.json(2 hunks)packages/fastify/tsconfig.json(1 hunks)packages/fastify/tsup.config.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/fastify/tsup.config.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/fastify/tsup.config.tspackages/fastify/tsconfig.jsonpackages/fastify/package.json
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/fastify/tsup.config.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/fastify/tsup.config.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/fastify/tsup.config.ts
packages/*/tsup.config.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
TypeScript compilation and bundling must use tsup.
Files:
packages/fastify/tsup.config.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/fastify/tsup.config.ts
packages/*/tsconfig.json
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Type checking must be performed with TypeScript and publint.
Files:
packages/fastify/tsconfig.json
packages/*/package.json
📄 CodeRabbit inference engine (.cursor/rules/global.mdc)
All publishable packages should be placed under the packages/ directory
packages/*/package.json: All publishable packages must be located in the 'packages/' directory.
All packages must be published under the @clerk namespace on npm.
Semantic versioning must be used across all packages.
Files:
packages/fastify/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
packages/fastify/tsconfig.json (1)
1-18: Configuration aligns with new flat output structure.The removal of
declarationDiris consistent with the updated export paths inpackage.json, where type declarations now reside directly indist/(e.g.,./dist/index.d.ts) instead ofdist/types/.packages/fastify/tsup.config.ts (3)
13-13: Correctly addresses the PR objective.Setting
minify: falseensures compatibility with static imports, as stated in the PR title. This is the core fix for the reported issue.
9-10: Output format and entry points align with package.json exports.The explicit entry points and dual format (
['cjs', 'esm']) correctly support the updated export structure inpackage.json, producing both.mjs(ESM) and.js(CJS) outputs.
6-6: Inconsistent DEV flag definition across monorepo requires verification.The change to
isWatchfor the__DEV__flag in packages/fastify is inconsistent with other packages in the monorepo. The DEV flag is actively used in runtime code (e.g., packages/shared/src/errors/clerkError.ts:69 for conditional documentation rendering), making this a behavioral change that affects multiple packages.Issue: Different packages define
__DEV__differently:
- Using
isWatch: fastify, backend, remix, react, shared, react-router, expo, express, expo-passkeys, chrome-extension- Using
!isProd: testing, elements, nextjs, tanstack-react-start, agent-toolkitSince packages/shared (which uses
isWatch) is a shared dependency imported by other packages, this inconsistency means packages may have divergent__DEV__values. Regular dev builds (without watch) will now have__DEV__ = false, differing from the previous!isProdbehavior.Confirm this change is intentional and align the
__DEV__logic across all packages.packages/fastify/package.json (2)
25-49: Export paths correctly follow Node.js dual-package conventions.The updated export paths properly distinguish between ESM (
.d.mts/.mjs) and CommonJS (.d.ts/.js) with correct type declaration extensions. The flatdist/structure aligns with the updated build configuration.
61-61: Manual verification required: Confirm lint:attw passes with the new export configuration.The sandbox environment cannot complete dependency installation due to a failing git-hosted package. Verify locally that
pnpm lint:attwpasses inpackages/fastify/with the--ignore-rules unexpected-module-syntaxflag removed. This confirms the new build configuration correctly resolves previous module syntax issues.
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
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.
Adjusted the exports since we removed the legacyOutput in tsup config. It should not be breaking since it only affects the internal build process, not the final output structure
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.
we are using the dts option in tsup config now
Description
This PR disables minification in tsup config (this is the only SDK that is minified somehow) to ensure readable exports for libraries like
tsx, and also removed thelegacyOutputoption.Fixes #6977
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit