Skip to content

feat(generate): allow param function to be composable #1292

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

vancegillies
Copy link

@vancegillies vancegillies commented Jun 3, 2025

Overview

Issue: #1290

This PR makes the return from the param function callable to allow composition while keeping the existing API stable.

Broken down into 3 parts

  • Types
    • Add a CallableWithParamsExpr to expr_WithParams which adds the function interface on the type level
    • helper paramsToInputArgs to map the param's into an object of $expr_Param or a TS type so you can call the same function from within a param callback or with normal ts types
      • Note here: you can't use base exprs you have to use primitive TS types or "params" that are passed to the param callback, so you can not for example partially apply params, but I don't think there is a valid use case for that
    • Helper paramOrTsType for making a union for the base TS type of a param expr
  • The param function
    • pretty simple, just assign the original return of the function to a callableExpr function that fulfills the above contract
  • The toEdgeQL function
    • Adds a check in the expr.__kind__ === ExpressionKind.WithParams branch of renderEdgeQLfunction, which is checking if the expr has any args and building a subquery for them.

Testing

I have done some little smoke tests, not everything works but I have a be able to get a basic composed query working here

import e from "./edgeql-js";

const testing1 = e.params(
  {
    arg1: e.uuid,
  },
  (params) => e.cast(e.str, params.arg1),
);

const testing2 = e.params(
  {
    arg2: e.uuid,
    arg3: e.optional(e.int16),
  },
  (params) => {
    const arg1 = testing1({
      arg1: params.arg2,
    });

    return e.set(e.select(arg1));
  },
);

const WITHOUT_ARGS = testing1.toEdgeQL();
const WITH_ARGS = testing1({
  arg1: "a216511e-78f4-4ccb-8492-3e5239b2d9db",
}).toEdgeQL();
const WITH_COMPOSE = testing2.toEdgeQL();
const COMPOSE_WITH_ARGS = testing2({
  arg2: "a216511e-78f4-4ccb-8492-3e5239b2d9db",
  arg3: 123,
}).toEdgeQL();

console.log(WITHOUT_ARGS);
console.log("");
console.log(WITH_ARGS);
console.log("");
console.log(WITH_COMPOSE);
console.log("");
console.log(COMPOSE_WITH_ARGS);

Which outputs

WITH
  __param__arg1 := <std::uuid>$arg1
SELECT (SELECT (<std::str>__param__arg1))

WITH
  __param__arg1 := <std::uuid>"a216511e-78f4-4ccb-8492-3e5239b2d9db"
SELECT (SELECT (<std::str>__param__arg1))

WITH
  __param__arg2 := <std::uuid>$arg2,
  __param__arg3 := <OPTIONAL std::int16>$arg3
SELECT (SELECT { (SELECT (WITH
  __param__arg1 := __param__arg2
SELECT (SELECT (<std::str>__param__arg1)))) })

WITH
  __param__arg2 := <std::uuid>"a216511e-78f4-4ccb-8492-3e5239b2d9db",
  __param__arg3 := <std::int16>123
SELECT (SELECT { (SELECT (WITH
  __param__arg1 := __param__arg2
SELECT (SELECT (<std::str>__param__arg1)))) })

Running these against a gel instance I get
Screenshot 2025-06-03 at 10 58 44 pm

TODO

  • Testing with more complex types and queries
  • Testing with client
  • ...

@gel-data-cla
Copy link

gel-data-cla bot commented Jun 3, 2025

All commit authors signed the Contributor License Agreement.
CLA signed

@vancegillies
Copy link
Author

@scotttrinh Would love some eyes on this to make sure I am heading in the right direction.

@scotttrinh
Copy link
Collaborator

@vancegillies I haven't looked at the code yet (I'll do that now), but just wanted to say just based on your description that you're going in the right direction here.

Remove the error that prevents 'withParams' from being used as a
nested expression.
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