Skip to content

docs(plans): gqlorm remove db import injection plan#1702

Draft
Tobbe wants to merge 1 commit into
mainfrom
tobbe-gqlorm-plan
Draft

docs(plans): gqlorm remove db import injection plan#1702
Tobbe wants to merge 1 commit into
mainfrom
tobbe-gqlorm-plan

Conversation

@Tobbe
Copy link
Copy Markdown
Member

@Tobbe Tobbe commented May 4, 2026

Adds implementation plan for removing the hardcoded src/lib/db import from gqlorm's Babel plugin, replacing it with smart detection of the user's existing db binding.

This is extracted from #1700

@netlify
Copy link
Copy Markdown

netlify Bot commented May 4, 2026

Deploy Preview for cedarjs failed.

Name Link
🔨 Latest commit 480ff66
🔍 Latest deploy log https://app.netlify.com/projects/cedarjs/deploys/69f8521c37b7a000088e24c3

@github-actions github-actions Bot added this to the next-release-patch milestone May 4, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 4, 2026

Greptile Summary

This PR adds a new implementation plan document for removing the hardcoded src/lib/db import from gqlorm's Babel plugin, replacing it with scope-based detection of the user's existing db binding. The plan is well-structured and the rationale (Babel over Vite for dual build-path coverage) is sound and matches the actual codebase.

  • The getTopLevelDbBinding sketch uses programPath.scope.getBinding('db'), which resolves bindings by local name. For import { db as prisma } from '@myorg/db', the local binding name is prisma, so getBinding('db') returns null and the plugin throws a hard error — contradicting test case 2 and the acceptance criteria that explicitly require aliased-import support. The implementation sketch needs to traverse ImportDeclaration nodes and match on imported.name === 'db' instead.

Confidence Score: 3/5

Safe to merge as a docs-only PR, but the implementation sketch has a logic flaw that will cause a hard build error for aliased imports if followed as written.

Only one file changed (a plan doc), so there is no runtime risk. However, the plan contains a P1 inconsistency: the getTopLevelDbBinding sketch doesn't handle aliased db imports despite the acceptance criteria and test cases explicitly requiring it.

docs/implementation-plans/gqlorm-remove-db-import-injection-plan.md — specifically the getTopLevelDbBinding implementation sketch around lines 120–128.

Important Files Changed

Filename Overview
docs/implementation-plans/gqlorm-remove-db-import-injection-plan.md New implementation plan for removing hardcoded src/lib/db injection; the getTopLevelDbBinding sketch is inconsistent with the aliased-import test case it claims to support.

Reviews (1): Last reviewed commit: "docs(plans): add gqlorm remove db import..." | Re-trigger Greptile

Comment on lines +120 to +128
---

## Changes

### 1. Rewrite `babel-plugin-cedar-gqlorm-inject` to detect existing `db` binding

**File:** `packages/babel-config/src/plugins/babel-plugin-cedar-gqlorm-inject.ts`

**Current behavior:** The plugin unconditionally injects:
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.

P1 getTopLevelDbBinding won't find aliased imports

The sketch calls programPath.scope.getBinding('db'), which looks up a binding by its local identifier name. For import { db as prisma } from '@myorg/db', Babel registers the binding under prisma (the local name), not db. So getBinding('db') returns null, the plugin throws the hard error, and the aliased-import case silently breaks — directly contradicting test case 2 and the acceptance criteria ("The plugin handles aliased imports").

To support aliasing, traverse ImportDeclaration nodes and find any specifier whose imported.name === 'db', then return local.name as the identifier to use in the Object.assign call.

@Tobbe Tobbe marked this pull request as draft May 4, 2026 08:06
Tobbe added a commit that referenced this pull request May 4, 2026
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.

1 participant