Skip to content

Conversation

@MH4GF
Copy link
Contributor

@MH4GF MH4GF commented Mar 25, 2025

Issue

  • resolve:

Why is this change needed?

To make past context easier for new developers to understand

What would you like reviewers to focus on?

Testing Verification

What was done

🤖 Generated by PR Agent at a78df6a

  • Added an ADR document for Node.js-based unified DB schema parsing.
  • Detailed the decision to use Node.js for schema parsing and ER diagram generation.
  • Outlined the use of WASM-compatible parsers for handling multiple schema formats.
  • Highlighted the benefits and trade-offs of the proposed approach.

Detailed Changes

Relevant files
Documentation
20241206-node-js-based-unified-db-schema-parsing.mdx
Added ADR for Node.js-based schema parsing approach           

frontend/apps/docs/content/docs/contributing/adr/20241206-node-js-based-unified-db-schema-parsing.mdx

  • Added a new ADR document for Node.js-based schema parsing.
  • Provided context, decision, and consequences for the approach.
  • Discussed the use of WASM-compatible parsers and unified data models.
  • Highlighted positive and negative impacts of the decision.
  • +57/-0   

    Additional Notes


    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @vercel
    Copy link

    vercel bot commented Mar 25, 2025

    The latest updates on your projects. Learn more about Vercel for Git ↗︎

    Name Status Preview Comments Updated (UTC)
    liam-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 25, 2025 6:27am
    liam-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 25, 2025 6:27am
    liam-erd-sample ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 25, 2025 6:27am
    3 Skipped Deployments
    Name Status Preview Comments Updated (UTC)
    test-liam-app ⬜️ Ignored (Inspect) Mar 25, 2025 6:27am
    test-liam-docs ⬜️ Ignored (Inspect) Mar 25, 2025 6:27am
    test-liam-erd-sample ⬜️ Ignored (Inspect) Mar 25, 2025 6:27am

    @changeset-bot
    Copy link

    changeset-bot bot commented Mar 25, 2025

    ⚠️ No Changeset found

    Latest commit: a78df6a

    Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

    This PR includes no changesets

    When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

    Click here to learn what changesets are, and how to add one.

    Click here if you're a maintainer who wants to add a changeset to this PR

    @supabase
    Copy link

    supabase bot commented Mar 25, 2025

    Updates to Preview Branch (add-parcing-adr) ↗︎

    Deployments Status Updated
    Database Tue, 25 Mar 2025 06:16:35 UTC
    Services Tue, 25 Mar 2025 06:16:35 UTC
    APIs Tue, 25 Mar 2025 06:16:35 UTC

    Tasks are run on every commit but only new migration files are pushed.
    Close and reopen this PR if you want to apply changes from existing seed or migration files.

    Tasks Status Updated
    Configurations Tue, 25 Mar 2025 06:16:44 UTC
    Migrations Tue, 25 Mar 2025 06:16:44 UTC
    Seeding Tue, 25 Mar 2025 06:16:44 UTC
    Edge Functions Tue, 25 Mar 2025 06:16:44 UTC

    View logs for this Workflow Run ↗︎.
    Learn more about Supabase for Git ↗︎.

    @MH4GF MH4GF marked this pull request as ready for review March 25, 2025 06:15
    @MH4GF MH4GF requested a review from a team as a code owner March 25, 2025 06:15
    @MH4GF MH4GF requested review from FunamaYukina, NoritakaIkeda, hoshinotsuyoshi and junkisai and removed request for a team March 25, 2025 06:15
    @qodo-free-for-open-source-projects
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Implementation Details

    The ADR mentions using WASM-compatible parsers but doesn't specify which specific parsers will be used for PostgreSQL DDL. Consider adding more concrete implementation details or examples of the parsers being considered.

    - For PostgreSQL DDL, we aim to use a WASM-compatible SQL parser tailored to PostgreSQL syntax.
    Missing Alternatives

    The ADR doesn't discuss alternative approaches that were considered and rejected. Adding a section on alternatives would strengthen the document by showing the decision-making process more clearly.

    ## Decision
    
    We will perform all DB schema parsing within a Node.js environment. Specifically:
    
    1. **Node.js as the Server-Side Runtime**
    
       - Use Node.js exclusively on the server side to perform schema parsing and ER diagram generation.
       - Deliver the rendered ER diagrams to the client via our web application (e.g., React Server Components) without running Node.js in the browser.
    
    2. **Existing Parsers**
    
       - For Prisma schemas (`schema.prisma`), we can leverage existing Node.js libraries.
       - For Rails’ `schema.rb`, we plan to use [ruby/prism](https://github.com/ruby/prism) in a form that can run under Node.js (e.g., via WASM).
       - For PostgreSQL DDL, we aim to use a WASM-compatible SQL parser tailored to PostgreSQL syntax.
    
    3. **Unified Data Model**
    
       - Map every schema format to a common `DBStructure` type so that subsequent ER diagram generation remains format-agnostic.
    
    4. **Future Growth**
       - If new formats emerge, we will first look for a WASM or JavaScript solution. Only if absolutely necessary will we consider introducing new runtimes or custom parser generators.
    

    @qodo-free-for-open-source-projects
    Copy link
    Contributor

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    Copy link
    Member

    @junkisai junkisai left a comment

    Choose a reason for hiding this comment

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

    There was nothing in the content that felt off! 🙌

    @junkisai junkisai added this pull request to the merge queue Mar 25, 2025
    @github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 25, 2025
    @MH4GF MH4GF added this pull request to the merge queue Mar 25, 2025
    Merged via the queue into main with commit 0053704 Mar 25, 2025
    23 checks passed
    @MH4GF MH4GF deleted the add-parcing-adr branch March 25, 2025 10:36
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    3 participants