--extract-request-params behavior depends on presence of query params

[david-crespo]

The procedure call template uses the presence of requestParams to decide whether to actually do the --extract-request-params behavior, i.e., put path and query params in a single object that is the first argument to the request function.

If requestParams is falsy, we essentially fall back to the default behavior.

swagger-typescript-api/templates/default/procedure-call.eta

Lines 24 to 34 in 7df956c

	const rawWrapperArgs = config.extractRequestParams ?
	_.compact([
	requestParams && {
	name: pathParams.length ? `{ ${_.join(pathParamsNames, ", ")}, ...${queryName} }` : queryName,
	optional: false,
	type: getInlineParseContent(requestParams),
	},
	...(!requestParams ? pathParams : []),
	payload,
	requestConfigParam,
	]) :

The problem (for me, anyway) is that requestParams is always null for a given route if the route takes no query params:

swagger-typescript-api/src/routes.js

Lines 295 to 302 in 7df956c

	const createRequestParamsSchema = ({
	queryParams,
	queryObjectSchema,
	pathArgsSchemas,
	extractRequestParams,
	routeName,
	}) => {
	if (!queryParams || !queryParams.length) return null;

So, for requests that only have path params but no query params, we will never generate a params object representing the path params, instead inlining them as individual arguments. I was able to confirm that commenting out if (!queryParams || !queryParams.length) return null; produces the desired behavior.

This behavior is counterintuitive given the documentation for the flag. It seems clear that both path params and query params are meant to be considered "request params", which means requestParams should not be null if there are path params.

  --extract-request-params      extract request params to data contract (default: false)
                                Also combine path params and query params into one object

If getting rid of that check would break the default extractRequestParams = false case, then maybe something like this would be appropriate:

- if (!queryParams || !queryParams.length) return null;
+ if (!extractRequestParams && (!queryParams || !queryParams.length)) return null;

[Bjorn-Eric-Abr]

This happens to me to - we only have path params and no query params, leaving us with no generated interface. Any plans on fixing this? I could do a PR and try @david-crespo s suggestion?

[david-crespo]

If you want to keep using this package without forcing a PR through, you can use the very neat patch-package to make the above change in your copy without having to maintain a fork and install directly from GitHub.

I did that for a little while, then I ended up writing my own generator by using @APIDevTools/swagger-parser to parse the spec and then I loop through it and make all the methods and stuff. It wasn't nearly as bad as I expected. The script is a few hundred lines long. I would link the repo but it's private. Will probably be public eventually.

It's a bunch of stuff like this:

/** write to file with newline */
function w(s: string) {
  console.log(s);
}

/** same as w() but no newline */
function w0(s: string) {
  process.stdout.write(s);
}


type Schema = OpenAPIV3.ReferenceObject | OpenAPIV3.SchemaObject;

type SchemaToTypeOpts = {
  propsInline?: boolean;
  name?: string;
  schemaNames?: string[]; // only here to pass to docComment
};

function schemaToType(schema: Schema, opts: SchemaToTypeOpts = {}) {
  if ("$ref" in schema) {
    w0(refToSchemaName(schema.$ref));
    return;
  }

  if (schema.type === "string") {
    if (schema.enum) {
      if (schema.enum.length === 1) {
        w0(`'${schema.enum[0]}'`);
      } else {
        for (const item of schema.enum) {
          w(`  | "${item}"`);
        }
      }
    } else if (
      schema.format === "date-time" &&
      opts.name?.startsWith("time_")
    ) {
      w0("Date");
    } else {
      w0("string");
    }
  } else if (schema.oneOf) {
    for (const prop of schema.oneOf) {
      w0("  | ");
      schemaToType(prop, { ...opts, propsInline: true });
    }
  } else if (schema.type === "array") {
    schemaToType(schema.items, opts);
    w0("[]");
  } else if (schema.allOf && schema.allOf.length === 1) {
    schemaToType(schema.allOf[0], opts);
  } else if (schema.type === "integer") {
    w0("number");
  } else if (schema.type === "boolean") {
    w0("boolean");
  } else if (schema.type === "object") {
    if (
      schema.additionalProperties &&
      typeof schema.additionalProperties === "object"
    ) {
      w0("Record<string, ");
      schemaToType(schema.additionalProperties, opts);
      w0(">");
    } else {
      // hack for getting cute inline object types for unions
      const suffix = opts.propsInline ? "" : "\n";
      w0("{ " + suffix);
      for (const propName in schema.properties) {
        const prop = schema.properties[propName];
        const nullable =
          ("nullable" in prop && prop.nullable) ||
          !schema.required ||
          !schema.required.includes(propName);

        if ("description" in prop) {
          docComment(prop.description, opts.schemaNames || []);
        }

        w0(snakeToCamel(propName));
        if (nullable) w0("?");
        w0(": ");
        schemaToType(prop, { ...opts, name: propName });
        if (nullable) w0(" | null");
        w0(", " + suffix);
      }
      w(" }");
    }
  } else if (typeof schema === "object" && Object.keys(schema).length === 0) {
    w0("any");
  } else {
    throw Error(`UNHANDLED SCHEMA: ${JSON.stringify(schema, null, 2)}`);
  }
}

//...

for (const schemaName in spec.components.schemas) {
  const schema = spec.components.schemas[schemaName];
  if ("description" in schema) {
    docComment(schema.description, schemaNames);
  }
  w0(`export type ${schemaName} =`);
  schemaToType(schema, { schemaNames });
  w("\n");

  if ("type" in schema && schema.type === "string" && schema.pattern) {
    w(`/** Regex pattern for validating ${schemaName} */`);
    w(`export const ${pascalToCamel(schemaName)}Pattern = `);
    // make pattern a string for now because one of them doesn't actually
    // parse as a regex. consider changing to `/${pattern}/` once fixed
    w(`"${schema.pattern}"\n`);
  }
}

[Bjorn-Eric-Abr]

Thanks for the detailed reply @david-crespo !
I will give the patch thing a go! Still, don't know if this project is actively maintained anymore? Might be a good idea to find another solution 👍

[vasilich6107]

@smorimoto could you pay attention to this bug report