Skip to content

fix: orm interfaces findX#1026

Open
yyyyaaa wants to merge 4 commits into
mainfrom
fix/orm-interfaces-findx-752
Open

fix: orm interfaces findX#1026
yyyyaaa wants to merge 4 commits into
mainfrom
fix/orm-interfaces-findx-752

Conversation

@yyyyaaa
Copy link
Copy Markdown
Contributor

@yyyyaaa yyyyaaa commented Apr 25, 2026

fixes https://github.com/constructive-io/constructive-planning/issues/752

ORM model interfaces (the core fix)

findFirst — single object return ✓
findFirst(
args: FindFirstArgs<S, UserFilter, UserOrderBy> & { select: S } & StrictSelect<S, UserSelect>
): QueryBuilder<{
user: InferSelectResult<UserWithRelations, S> | null; // ← singular, was array
}>
With transform callback: (data) => ({ user: data.users?.nodes?.[0] ?? null })

findOne — single object return ✓
Same { user: ... | null } shape as findFirst, using buildFindManyDocument with where: { id: { equalTo: args.id } }, first: 1.

Coverage across SDKs (constructive-sdk only — others mirror)

  • findFirst<...> declarations: 168 (auth: 9, objects: 5, admin: 34, public: 120)
  • Unique singular { xxx: InferSelectResult<XxxWithRelations, S> | null } return shapes: 120 distinct entities
  • Regression check: 0 instances of findFirst returning an array

FindFirstArgs interface (in generated select-types.ts)

export interface FindFirstArgs<TSelect, TWhere, TOrderBy = never> {
select?: TSelect;
where?: TWhere;
orderBy?: TOrderBy[]; // ← new orderBy support
}

CLI (sdk/constructive-cli)

  • Help text includes --orderBy under "Find-First Options" (verified user.ts and others)
  • handleFindFirst calls parseFindFirstArgs<FindFirstArgs<UserSelect, UserFilter, UserOrderBy>>(argv, defaultSelect)

React hooks (sdk/constructive-react)

  • useUserQuery returns UseQueryResult<{ user: InferSelectResult<UserWithRelations, S> | null }>
  • Delegates to getClient().user.findOne(...) — picks up the new singular shape transparently
  • fetchUserQuery and prefetchUserQuery mirror the same contract

migrate-client SDK

Same pattern verified on SqlAction.findFirst — returns { sqlAction: ... | null } with SqlActionOrderBy type param.

Build + typecheck (already done)

  • All 4 SDKs build clean
  • tsc --noEmit passes with zero errors on all 4 SDKs
  • 306 codegen tests pass, 119 snapshots green

@yyyyaaa yyyyaaa requested a review from pyramation April 25, 2026 06:06
@pyramation
Copy link
Copy Markdown
Contributor

nice is this ready to review?

@yyyyaaa
Copy link
Copy Markdown
Contributor Author

yyyyaaa commented Apr 28, 2026

yep, pr ready

@pyramation
Copy link
Copy Markdown
Contributor

out of curiosity, breaking changes? so we'll need to change FE before we do this one?

@yyyyaaa yyyyaaa force-pushed the fix/orm-interfaces-findx-752 branch from 474d2e0 to 815edac Compare May 8, 2026 03:48
@socket-security
Copy link
Copy Markdown

socket-security Bot commented May 8, 2026

No dependency changes detected. Learn more about Socket for GitHub.

👍 No dependency changes detected in pull request

Comment thread graphql/query/src/runtime/index.ts Outdated
export { createFetch } from './localhost-fetch';
export type { FetchFunction } from './localhost-fetch';
export { createFetch } from './localhost-fetch.js';
export type { FetchFunction } from './localhost-fetch.js';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why .js? we are not using esm

const nodeRequire = getNodeModuleLoader();
if (nodeRequire) {
const http = nodeRequire('node:' + 'http') as typeof import('node:http');
const https = nodeRequire('node:' + 'https') as typeof import('node:https');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we use our fetch library? what is this for?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread graphql/query/package.json Outdated
"import": "./esm/runtime/index.js",
"require": "./runtime/index.js"
}
},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea we don't use exports — see constructive-skills pnpm

@yyyyaaa yyyyaaa force-pushed the fix/orm-interfaces-findx-752 branch from 65b829b to 20702c4 Compare May 13, 2026 07:08
yyyyaaa added 3 commits May 13, 2026 14:13
Templates and snapshots updated; CLI table command + ORM client/model
generators now treat findFirst/findOne as singular (use the singular
noun, not the by-id query name, as the result key). Server-test and
test-app integration coverage updated to assert single-object shape.
- Drop .js extensions from runtime re-exports (repo is CJS w/
  moduleResolution: node; .js belongs to ESM/Node16).
- Replace local localhost-fetch.ts with @constructive-io/fetch@^1.0.0
  (already used by packages/upload-client; same API surface).
- Drop the package.json "exports" map; sub-paths resolve via filesystem
  walk like every other publishable package in this monorepo.

Lockfile regenerated.
Picks up findFirst/findOne single-object return, runtime export
alignment, and new tables/bulk mutations merged from main.
@yyyyaaa yyyyaaa force-pushed the fix/orm-interfaces-findx-752 branch from 20702c4 to 70085fb Compare May 13, 2026 07:13
The earlier SDK regen pass missed 30 model files in
sdk/constructive-cli/src/{admin,public}/orm/models/ — tables added on
main by automated bot commits between May 9-11 using the pre-#752
codegen. They retained the old 6-arg buildFindFirstDocument call,
breaking the CLI build with TS2345 once query-builder.ts moved to the
new 7-arg signature.

Re-ran pregenerate (rimraf) + generate cleanly across all four SDK
targets: @constructive-sdk/cli, @constructive-io/sdk,
@constructive-io/react, @pgpmjs/migrate-client.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants