-
Notifications
You must be signed in to change notification settings - Fork 10
Introduce vendor namespace support across ThunderID SDKs #24
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
Open
brionmario
wants to merge
1
commit into
thunder-id:main
Choose a base branch
from
brionmario:flexible-vendor-config
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json | ||
| language: en-US | ||
|
|
||
| reviews: | ||
| request_changes_workflow: false | ||
| high_level_summary: true | ||
| poem: false | ||
|
|
||
| path_instructions: | ||
| - path: "packages/**/*.{ts,tsx}" | ||
| instructions: | | ||
| Flag any hardcoded occurrence of the literal `thunderid`/`ThunderID`/`THUNDERID` (any casing) used to | ||
| build a runtime key or name — storage keys, cookie/session names, CSS class prefixes, | ||
| `useState`/composable keys, log tags, DOM `id`/`data-*` attributes, and similar. | ||
|
|
||
| Do not flag the package's entry point — the main client class, provider component, or a file whose | ||
| purpose IS to represent the SDK itself (e.g. `ThunderIDClient`, `ThunderIDProvider`, | ||
| `ThunderIDBrowserClient`). That's a fixed identity, not a per-tenant value. | ||
|
|
||
| For everything else, ask: **"Should this read from the `vendor` config / `getVendorPrefix()` util | ||
| instead of hardcoding the vendor name?"** The best fix is avoiding the vendor name entirely when the | ||
| brand prefix isn't load-bearing. When a brand-scoped namespace is genuinely required, it should resolve | ||
| through `getVendorPrefix(vendor)` (from `@thunderid/javascript`, re-exported via | ||
| `@thunderid/browser`/`@thunderid/node`) rather than a literal string or a repeated inline | ||
| `vendor ?? 'thunderid'` fallback. | ||
|
|
||
| - path: "packages/**/*.{ts,tsx}" | ||
| instructions: | | ||
| This is a layered monorepo: `@thunderid/javascript` is the framework-agnostic core; `@thunderid/browser` | ||
| and `@thunderid/node` build on it; framework packages (`react`, `vue`, `nextjs`, `nuxt`, `express`, | ||
| `react-router`, `tanstack-router`) build on those. If this change adds a utility/helper/constant that | ||
| duplicates logic already present in another package, or that more than one framework package would | ||
| plausibly need, ask: **"Should this live lower in the package hierarchy (e.g. `@thunderid/javascript`) | ||
| and be re-exported instead?"** | ||
|
|
||
| auto_review: | ||
| enabled: true | ||
| ignore_title_keywords: | ||
| - "WIP" | ||
| - "DO NOT MERGE" | ||
| base_branches: | ||
| - main | ||
|
|
||
| path_filters: | ||
| - "!**/dist/**" | ||
| - "!**/node_modules/**" | ||
| - "!**/*.d.ts" | ||
| - "!**/.next/**" | ||
| - "!**/.nuxt/**" | ||
| - "!**/.output/**" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| # ThunderID JavaScript SDKs — Agent Instructions | ||
|
|
||
| ## Project overview | ||
|
|
||
| pnpm + Turborepo monorepo of TypeScript SDKs under `packages/`: a framework-agnostic core | ||
| (`@thunderid/javascript`), a browser layer (`@thunderid/browser`), a server layer (`@thunderid/node`), and | ||
| framework SDKs built on top of those (`react`, `vue`, `nuxt`, `nextjs`, `express`, `react-router`, | ||
| `tanstack-router`). `samples/<framework>/quickstart` directories contain standalone demo apps — not part of | ||
| any published package. | ||
|
|
||
| ## Build & test | ||
|
|
||
| ```bash | ||
| pnpm install | ||
|
|
||
| pnpm build # turbo run build across all packages | ||
| pnpm test # turbo run test | ||
| pnpm lint # turbo run lint (@thunderid/eslint-plugin) | ||
| pnpm typecheck # turbo run typecheck | ||
| pnpm format:check | ||
|
|
||
| # Scope to a single package: | ||
| pnpm --filter @thunderid/<pkg> run build | ||
| pnpm --filter @thunderid/<pkg> run test | ||
| ``` | ||
|
|
||
| ## Package hierarchy — check before adding a util | ||
|
|
||
| `@thunderid/javascript` is the framework-agnostic core (auth flows, token management, i18n, `VendorConstants`, | ||
| shared utils). Every other package depends on it, directly or transitively: | ||
|
|
||
| - `@thunderid/browser` → `@thunderid/javascript` + DOM/browser APIs (storage, session sync, WebAuthn). | ||
| - `@thunderid/node` → `@thunderid/javascript` + server-side session/cookie handling. | ||
| - `@thunderid/react`, `@thunderid/vue` → `@thunderid/browser`. | ||
| - `@thunderid/nextjs` → `@thunderid/node` + `@thunderid/react`. | ||
| - `@thunderid/nuxt` → `@thunderid/node` + `@thunderid/vue`. | ||
| - `@thunderid/express` → `@thunderid/node`. | ||
| - `@thunderid/react-router`, `@thunderid/tanstack-router` → `@thunderid/react`. | ||
|
|
||
| **Before adding a helper/util/constant to a framework package, check whether it belongs lower in this | ||
| hierarchy instead.** If the same logic would be needed by more than one framework package (or you're about to | ||
| copy-paste one), it almost certainly belongs in `@thunderid/javascript` — or `@thunderid/browser`/ | ||
| `@thunderid/node` if it needs DOM/server APIs — and should be re-exported from there, not duplicated. Every | ||
| package's `src/index.ts` re-exports its dependency's exports (`export * from '@thunderid/javascript'`, etc.), | ||
| so check there before assuming a helper isn't already available. | ||
|
|
||
| ## Vendor naming rules | ||
|
|
||
| The SDK is white-labelable: a consuming app can override the brand/vendor namespace via the `vendor` config | ||
| field, so storage keys, cookie names, CSS class prefixes, state keys, etc. shouldn't be pinned to one brand. | ||
|
|
||
| - Do not hardcode the literal `thunderid`/`ThunderID`/`THUNDERID` (any casing) when building a runtime | ||
| key/name that a consumer's `vendor` override should control — storage keys, cookie/session names, CSS | ||
| class prefixes, `useState`/composable keys, log tags, DOM `id`/`data-*` attributes, and the like. | ||
| - It's fine for the **entry point** — the main client class, provider component, or a file whose purpose IS | ||
| to represent the SDK itself (e.g. `ThunderIDClient`, `ThunderIDProvider`, `ThunderIDBrowserClient`) — to | ||
| carry the name. That's a fixed identity, not a per-tenant value. Don't flag those. | ||
| - Avoiding the vendor name entirely is the best outcome, when the brand prefix isn't actually load-bearing. | ||
| - When a brand-scoped namespace is genuinely required, resolve it through `getVendorPrefix(vendor)` (defined | ||
| in `@thunderid/javascript`, re-exported via `@thunderid/browser`/`@thunderid/node`) instead of hardcoding a | ||
| literal or repeating an inline `vendor ?? 'thunderid'` fallback. That default belongs in one place. | ||
|
|
||
| ## Code style | ||
|
|
||
| - TypeScript, ESLint via `@thunderid/eslint-plugin`, Prettier. | ||
| - Every source file carries a copyright header (`@thunderid/copyright-header` ESLint rule); `samples/**` is | ||
| exempt (see `eslint.config.js`). | ||
| - Prefer editing or re-exporting an existing util over duplicating a helper across packages (see hierarchy | ||
| above). | ||
|
|
||
| ## File layout | ||
|
|
||
| ``` | ||
| packages/ | ||
| javascript/ Framework-agnostic core SDK | ||
| browser/ Browser/DOM layer (storage, WebAuthn, session sync) | ||
| node/ Server-side layer (cookies, sessions) | ||
| react/ React components + hooks | ||
| vue/ Vue components + composables | ||
| nextjs/ Next.js SDK (client + server) | ||
| nuxt/ Nuxt module | ||
| express/ Express middleware | ||
| react-router/ React Router integration | ||
| tanstack-router/ TanStack Router integration | ||
| samples/ Standalone quickstart demo apps, one per framework | ||
| ``` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| @AGENTS.md |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use
getVendorPrefix(vendor)instead of repeating the inline fallback.The coding guidelines explicitly say to resolve vendor namespaces with
getVendorPrefix(vendor)rather than repeating an inlinevendor ?? 'thunderid'fallback. Line 101 does exactly this pattern. Additionally,(fullConfig as any).vendorbypasses type safety — ifvendoris now a supported config option, it should be declared on theAuthClientConfigtype so theas anycast is unnecessary.As per coding guidelines: "When a brand-scoped namespace is genuinely required, resolve it with
getVendorPrefix(vendor)rather than hardcoding a brand literal or repeating an inlinevendor ?? 'thunderid'fallback."♻️ Proposed refactor
If
vendoris added toAuthClientConfig<T>, the cast can be removed entirely:📝 Committable suggestion
🧰 Tools
🪛 ESLint
[error] 101-101: Unsafe assignment of an
anyvalue.(
@typescript-eslint/no-unsafe-assignment)[error] 101-101: Unexpected any. Specify a different type.
(
@typescript-eslint/no-explicit-any)[error] 101-101: Unsafe member access .vendor on an
anyvalue.(
@typescript-eslint/no-unsafe-member-access)[error] 103-103: Unsafe argument of type
anyassigned to a parameter of typestring.(
@typescript-eslint/no-unsafe-argument)🤖 Prompt for AI Agents
Source: Coding guidelines