openapi-typescript icon indicating copy to clipboard operation
openapi-typescript copied to clipboard

fix(openapi-typescript): --make-paths-enum renames the paths URL

Open insertmike opened this issue 9 months ago • 11 comments
trafficstars

Changes

Fixes: https://github.com/openapi-ts/openapi-typescript/issues/2101, when the --make-paths-enum was reintroduced back it was reintroduced with previously fixed bug: https://github.com/openapi-ts/openapi-typescript/issues/950

How to Review

It's straight-forward change :)

Checklist

  • [x] Unit tests updated
  • [x] docs/ updated (if necessary)
  • [x] pnpm run update:examples run (only applicable for openapi-typescript)

insertmike avatar Feb 13 '25 15:02 insertmike

Deploy request for openapi-ts accepted.

Name Link
Latest commit 21ab4c0464255aa844eef5d7e809bd95cf1a401e
Latest deploy log https://app.netlify.com/projects/openapi-ts/deploys/67ae0f550dd5640008f3bab5

netlify[bot] avatar Feb 13 '25 15:02 netlify[bot]

🦋 Changeset detected

Latest commit: e60c1693172c33e5850196b7e3872f4a1d757a1c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
openapi-typescript Patch

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

changeset-bot[bot] avatar Feb 13 '25 15:02 changeset-bot[bot]

@insertmike I also opened a PR #2153 a few minutes ago! Thanks for the reminder :) As for the fix, it's up to the maintainers but the adaptedUrl line can be completely removed.

semanser avatar Feb 13 '25 15:02 semanser

@Semigradsky You are right, I did not look into the code too much, then the maintainers can feel free to close my PR and merge yours #2153 but please take some action, it's straight-forward change and the community needs it 🙏

I will leave this PR open to bring back awareness to this issue..

insertmike avatar Feb 13 '25 15:02 insertmike

@semanser @insertmike great minds think alike 😅

theo-staizen avatar Feb 13 '25 20:02 theo-staizen

Hi all. We are happy to take a look at this, but please refrain from directly mentioning maintainers unless it's absolutely critical. Thank you!

htunnicliff avatar Feb 14 '25 00:02 htunnicliff

Noted - won't happen again. Thanks for your time :)

insertmike avatar Feb 14 '25 08:02 insertmike

@drwpow it looks like this does fix a reintroduced bug. However, applying it will affect the behavior for anyone who has started using the flag. I know you mentioned the following when this was fixed, so I want to double-check before we change its behavior:

since it’s an opt-in feature flag I support adding it! (I’ll only try to ensure that default behavior doesn’t change).

Do you think a patch release is appropriate?

htunnicliff avatar Feb 15 '25 00:02 htunnicliff

@htunnicliff Looking back in time, when this feature was first introduced, it was implemented incorrectly (issue) and was patched with this Pull Request. I suspect that when reintroducing the feature, the author of the pull request simply copied the call from the original implementation without realizing that it was initially incorrect and patched in time.

We could add it as a flag, but to me, it just seems fundamentally wrong—ApiPaths is not useful if it doesn’t match the original paths of the OpenAPI schema. WDYT?

insertmike avatar Feb 15 '25 09:02 insertmike

@theo-staizen @semanser

Temporary Workaround for the --make-paths-enum Bug

Assuming you saved the snippet as postprocess.js:

/**
 * Workaround for --make-paths-enum bug:
 * https://github.com/openapi-ts/openapi-typescript/pull/2152
 *
 * Generated ApiPaths enum values use ":id" instead of "{id}".
 * This script converts ":param" to "{param}".
 *
 * Usage: node posprocessOpenApiGeneration.js <generated-file-path>
 */
import fs from "fs";

// Get file path from command-line arguments
const filePath = process.argv[2];

if (!filePath) {
  console.error("Usage: node postprocess.js <file-path>");
  process.exit(1);
}

let content = fs.readFileSync(filePath, "utf8");

// Regex to capture the ApiPaths enum block
// Group 1: "export enum ApiPaths {"
// Group 2: the enum body (non-greedy)
// Group 3: the closing brace
const enumRegex = /(export\s+enum\s+ApiPaths\s*{)([\s\S]*?)(\n?\s*})/;

// Replace only inside the ApiPaths enum block
content = content.replace(enumRegex, (match, start, enumBody, end) => {
  // Replace occurrences of ":param" with "{param}" in the enum body
  const modifiedBody = enumBody.replace(/:([a-zA-Z0-9_]+)/g, "{$1}");
  return `${start}${modifiedBody}${end}`;
});

fs.writeFileSync(filePath, content, "utf8");
console.log(`File "${filePath}" updated successfully!`);

Add a post-generation script to your package.json so that the patch is applied automatically after generating your API client:

{
  "scripts": {
    "script-generating-openapi": "your-openapi-generation-command OUTPUT_PATH",
    "postscript-generating-openapi": "node postprocess.js OUTPUT_PATH"
  }
}

When you run the script-generating-openapi command, the post script will automatically be invoked, updating your ApiPaths enum to use the correct placeholder syntax.

insertmike avatar Feb 18 '25 08:02 insertmike

@insertmike coming here from #2164. I realize that we should release this soon, but I do not want to overrule ongoing discussions between maintainers. (IMO this clearly is a bugfix, but that's only one opinion).

gzm0 avatar Mar 09 '25 16:03 gzm0

@htunnicliff can you aprove and release this really bother me.

igieon avatar Mar 18 '25 16:03 igieon

I hope that we soon come to get on the master train :)

insertmike avatar Apr 07 '25 08:04 insertmike