Seed @thunderid/configure-applications - #4445
Merged
DonOmalVindula merged 1 commit intoJul 31, 2026
Merged
Conversation
Contributor
|
Important Review skippedToo many files! This PR contains 138 files, which is 38 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (138)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Stand up the applications package and move in only the transitive closure that the flows feature imports: the useGetApplication and useGetApplications hooks, the application models they depend on, and the shared query keys. The feature's remaining files stay in the console and migrate later into the package that now exists. This is the second step of extracting the flow builder into frontend/packages. The builder's simulation preview reads application data, and a package cannot import @/features/..., so those hooks have to live in a package before the builder can move. Seeding with api, models and constants keeps the dependency graph acyclic: none of them import flows, so configure-applications does not depend on the builder while the builder depends on it. Moving application-query-keys now keeps cache keys identical between the two packages, which would silently diverge if duplicated. Importers are repointed at the package rather than kept working through re-export shims. The two moved hook tests adopt the mock style the other packages use, since they now run under the packages' browser-mode vitest. Refs thunder-id#4425
DonOmalVindula
force-pushed
the
seed-configure-applications
branch
from
July 31, 2026 03:41
cad0f64 to
e57baa9
Compare
5 tasks
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
DonOmalVindula
enabled auto-merge
July 31, 2026 04:46
JayaShakthi97
approved these changes
Jul 31, 2026
12 tasks
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Purpose
Second step toward extracting the flow builder into
frontend/packages/(see #4425).The builder's
SimulationStepPreviewreads application data (useGetApplications+useGetApplication), andgatePreviewTransformsneeds theApplicationtype. A package cannot import@/features/..., so those have to live in a package before the builder can move.This stands up
@thunderid/configure-applicationsand seeds it with only the transitive closure the builder needs — 8 files. The feature's remaining ~220 files stay in the console and migrate later into the package that now exists.Why this seeding keeps the graph acyclic
applicationsandflowsimport each other, so extracting both naively would be circular:applications → flows: 8 modules (the flows-settings tab, create-application signin options)flows → applications: the simulation preview's application dataThe seed avoids the cycle because
applications/api/,models/andconstants/contain zero flows imports — all 17 flows-importing files live inpages/,contexts/andcomponents/, none of which move here. Soconfigure-applicationsdoes not depend on the builder while the builder depends on it.Verified with a cold-cache
turbo run build --filter=./packages/*: 22 tasks ordered successfully. Turbo failing to order the builds is the cycle detector.Approach
What moved (the closure of
useGetApplication,useGetApplicationsandApplication):api/useGetApplication.tsapi/useGetApplications.tsmodels/application.tsmodels/responses.tsmodels/inbound-auth.tsmodels/oauth.tsmodels/token.tsconstants/application-query-keys.tsThe three
models/{inbound-auth,oauth,token}files come along only becauseapplication.tsimports them — part of the closure, not a scope expansion. All 8 have tests, which moved too. Depth is preserved, so all 16 files register as renames and the moved files' relative imports resolve unchanged.Moving
application-query-keysnow is deliberate: it keeps react-query cache keys identical betweenconfigure-flowsand the rest ofconfigure-applications, which would silently diverge if duplicated.Scaffolding came from the repo's own
packages/createfeature template. The same two template corrections as #4404 were needed: the template emits a populateddependenciesblock where every real package usesdependencies: {}with everything in peers (this matters becauserolldown.config.jsderivesexternalfrom deps + peers), and it references the stale@asgardeo/reactscope. Both worth fixing in the template itself.Peers are exactly what the 8 files import:
@tanstack/react-query,@thunderid/contexts,@thunderid/react,react.No re-export shims (
AGENTS.mdforbids them), so importers were repointed directly: 150 import statements across 99 files, plus 12vi.mock/dynamic-importsites.That rewrite was done by resolving every specifier to an absolute path and rewriting only genuine hits, rather than by pattern-matching module names. That mattered:
flowshas its ownmodels/responses, and a naivemodels/responses'pattern would have wrongly rewritten it. It also revealed my file-count estimate was low — the real scope was 99 files, not the ~44 my grep patterns suggested, because they missed deeper relative paths like../../../../models/oauth.Mock conversions needed care, since the package exports named rather than default and mocking a whole package replaces all its exports:
vi.mock('<module>')andvi.mock('<module>', () => ({default: fn}))became a singlevi.mock('@thunderid/configure-applications', async importOriginal => ({...(await importOriginal()), <name>: fn}))per file, so the package's other exports survive.SimulationStepPreview.test.tsxmocked two moved modules; those merged into one package mock with both overrides.const {default: X} = await import(...)becameconst {<name>: X} = await import(...).Related Issues
Related PRs
configure-verifiable-presentations)Checklist
breaking changelabel added.Security checks
Verification
build,typecheck,lintclean, and 8 test files / 78 tests pass.tsc -bclean, 478 test files / 7562 tests pass, eslint 0 errors, production build succeeds.turbo run build --filter=./packages/*: 22/22, no cycle.pnpm install --frozen-lockfile: clean (39 workspace projects).flowsno longer imports anything fromapplicationsexceptGradientBorderButton, which is the next PR's job.