MAJOR 4.0.0 - use wasm libpg-query #397
Conversation
🦋 Changeset detectedLatest commit: 55ded03 The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
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 Git ↗︎ 1 Skipped Deployment
|
WalkthroughThis update migrates the SafeQL project to use WebAssembly (WASM) builds of the Changes
Sequence Diagram(s)sequenceDiagram
participant ESLintRule as ESLint Rule
participant Codegen as Codegen (generateSyncE)
participant WASMParser as WASM libpg-query
ESLintRule->>Codegen: generateSyncE(query, ...)
Codegen->>WASMParser: parser.parse(query.text)
WASMParser-->>Codegen: Parsed AST
Codegen-->>ESLintRule: Generation Result
sequenceDiagram
participant User as User
participant SafeQL as SafeQL Package
participant WASMParser as WASM libpg-query
User->>SafeQL: Provide SQL Query
SafeQL->>WASMParser: parser.parse(query)
WASMParser-->>SafeQL: Parsed AST
SafeQL-->>User: Result (analysis, codegen, etc.)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/blog/safeql-v5-roadmap.md (1)
122-122: Reduce repetitive sentence starts in closing paragraph
The closing lines begin multiple sentences with “I”/“I’m.” Consider varying sentence openings for better flow (e.g., start one sentence with “Excited” or “Looking forward”)..changeset/forty-meals-behave.md (1)
6-6: Fix typos in changeset documentation.The changeset correctly documents the breaking change, but contains typos that should be corrected for professional documentation.
Apply this diff to fix the typos:
-BREAKING: SafeQL now requires the latest minore releases of `libpg-query` which use WASM builds instead of native binaries. This change imporves compatibility across different platforms and eliminates native compilation issues. +BREAKING: SafeQL now requires the latest minor releases of `libpg-query` which use WASM builds instead of native binaries. This change improves compatibility across different platforms and eliminates native compilation issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
docs/public/safeql-v4-roadmap.jpgis excluded by!**/*.jpgdocs/public/safeql-v5-roadmap.jpgis excluded by!**/*.jpgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
.changeset/forty-meals-behave.md(1 hunks)README.md(1 hunks)docs/blog/safeql-v5-roadmap.md(4 hunks)docs/guide/getting-started.md(1 hunks)packages/eslint-plugin/package.json(1 hunks)packages/eslint-plugin/src/rules/check-sql.rule.ts(2 hunks)packages/eslint-plugin/src/workers/check-sql.worker.ts(1 hunks)packages/eslint-plugin/src/workers/index.ts(0 hunks)packages/eslint-plugin/src/workers/parse-sync.worker.ts(0 hunks)packages/generate/package.json(1 hunks)packages/generate/src/generate.test.ts(2 hunks)packages/generate/src/generate.ts(2 hunks)packages/generate/src/utils/get-nonnullable-columns.test.ts(1 hunks)packages/generate/src/utils/get-relations-with-joins.test.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/eslint-plugin/src/workers/parse-sync.worker.ts
- packages/eslint-plugin/src/workers/index.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/generate/src/generate.test.ts (1)
packages/shared/src/errors.ts (1)
InternalError(132-167)
🪛 LanguageTool
docs/blog/safeql-v5-roadmap.md
[style] ~122-~122: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...the library more flexible and powerful. I'm looking forward to hearing your feedb...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (19)
README.md (1)
36-36: Simplified Prerequisites Aligns with WASM Migration
The removal of native build requirements and consolidation into a single ESLint setup step correctly reflects the shift to the WASM-basedlibpg-query.docs/guide/getting-started.md (1)
9-9: Streamlined Prerequisites Section
Converting the numbered list into a concise sentence improves readability and maintains consistency with the README.docs/blog/safeql-v5-roadmap.md (8)
2-2: Document title updated to v5
The title correctly reflects the new major version.
16-16: OG meta title reflects v5
The Open Graph title has been updated accordingly.
19-19: Verify OG image availability
You’ve updated the OG image URL to the v5 roadmap. Please confirm thathttps://safeql.dev/safeql-v5-roadmap.jpgis live and accessible to avoid broken social previews.
22-22: OG URL updated to v5 blog path
The Open Graph URL now points to the v5 roadmap. Ensure this matches your deployed route.
25-25: OG description updated for v5
The metadata description aligns with the updated version.
31-31: Heading updated to v5
The top-level heading reflects the new major release.
35-35: Inline image reference updated
The embedded image path has been changed for v5. Verify that/safeql-v5-roadmap.jpgis correctly placed in the static assets.
51-51: Deprecation note points to v5 config file
The prose correctly signals deprecation of ESLint-based config in favor ofsafeql.config.tsfor v5.packages/generate/package.json (1)
37-37: LGTM: Dependency update aligns with WASM migration.The update to
libpg-queryv17.5.2 correctly reflects the migration from native binaries to WASM builds as documented in the PR objectives.packages/eslint-plugin/package.json (1)
54-54: LGTM: Consistent dependency update across packages.The libpg-query version update to v17.5.2 is consistent with the generate package, ensuring compatibility across the SafeQL ecosystem.
packages/generate/src/utils/get-nonnullable-columns.test.ts (1)
82-82: LGTM: API change correctly implements new parser interface.The change from
parser.parseQuerytoparser.parsealigns with the libpg-query v17 WASM-based API.packages/generate/src/generate.test.ts (2)
7-7: LGTM: Import style correctly updated for WASM API.The change from named import to default import aligns with the libpg-query v17 WASM-based API structure.
171-171: LGTM: Parser API usage correctly updated.The change from
parseQuerytoparser.parseproperly implements the new WASM-based parsing interface.packages/generate/src/utils/get-relations-with-joins.test.ts (1)
119-119: LGTM! Parser method updated correctly.The change from
parser.parseQuerytoparser.parsealigns with the new libpg-query API and is consistent with similar changes across the codebase.packages/eslint-plugin/src/workers/check-sql.worker.ts (1)
25-36: Good architectural improvement - parsing centralized in generate package.The removal of
pgParsedfrom the worker interface and the corresponding import cleanup aligns well with centralizing SQL parsing within the generate package. This simplifies the worker's responsibilities and reduces coupling.packages/generate/src/generate.ts (1)
25-25: Excellent centralization of SQL parsing logic.Moving the parsing responsibility into the
generatefunction is a clean architectural improvement. The async parsing withawait parser.parse(query.text)properly handles the WASM-based parser implementation.Also applies to: 263-265
packages/eslint-plugin/src/rules/check-sql.rule.ts (1)
179-181: Clean removal of SQL parsing from the ESLint rule.The simplified call to
generateSyncEwithout pre-parsing aligns perfectly with the architectural change to centralize parsing in the generate package. This reduces the rule's complexity and responsibilities.
Summary by CodeRabbit