Skip to content

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

Merged
gzm0 merged 5 commits into
openapi-ts:mainfrom
mihailyonchev:main
Apr 7, 2025
Merged

fix(openapi-typescript): --make-paths-enum renames the paths URL#2152
gzm0 merged 5 commits into
openapi-ts:mainfrom
mihailyonchev:main

Conversation

@mihailyonchev

@mihailyonchev mihailyonchev commented Feb 13, 2025

Copy link
Copy Markdown
Contributor

Changes

Fixes: #2101, when the --make-paths-enum was reintroduced back it was reintroduced with previously fixed bug: #950

How to Review

It's straight-forward change :)

Checklist

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

@mihailyonchev
mihailyonchev requested a review from a team as a code owner February 13, 2025 15:27
@netlify

netlify Bot commented Feb 13, 2025

Copy link
Copy Markdown

👷 Deploy request for openapi-ts accepted.

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

@changeset-bot

changeset-bot Bot commented Feb 13, 2025

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: e60c169

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

@mihailyonchev mihailyonchev changed the title [transform/paths] Remove parameters replacing [transform/paths] Fix --make-paths-enum renames the paths URL Feb 13, 2025
@semanser

Copy link
Copy Markdown

@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.

@mihailyonchev

mihailyonchev commented Feb 13, 2025

Copy link
Copy Markdown
Contributor Author

@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..

@mihailyonchev mihailyonchev changed the title [transform/paths] Fix --make-paths-enum renames the paths URL fix(transform/paths): --make-paths-enum renames the paths URL Feb 13, 2025
@mihailyonchev mihailyonchev changed the title fix(transform/paths): --make-paths-enum renames the paths URL fix(openapi-typescript): --make-paths-enum renames the paths URL Feb 13, 2025
@theo-staizen

Copy link
Copy Markdown

@semanser @insertmike great minds think alike 😅

@htunnicliff

Copy link
Copy Markdown
Contributor

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!

@mihailyonchev

Copy link
Copy Markdown
Contributor Author

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

@htunnicliff

Copy link
Copy Markdown
Contributor

@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?

@mihailyonchev

Copy link
Copy Markdown
Contributor Author

@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?

@mihailyonchev

mihailyonchev commented Feb 18, 2025

Copy link
Copy Markdown
Contributor Author

@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.

@mihailyonchev mihailyonchev mentioned this pull request Feb 20, 2025
@gzm0

gzm0 commented Mar 9, 2025

Copy link
Copy Markdown
Contributor

@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).

@duncanbeevers duncanbeevers left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I support releasing this as a patch release.

@igieon

igieon commented Mar 18, 2025

Copy link
Copy Markdown

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

@mihailyonchev

Copy link
Copy Markdown
Contributor Author

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

@gzm0
gzm0 merged commit 47e4b5e into openapi-ts:main Apr 7, 2025
@openapi-ts-bot openapi-ts-bot mentioned this pull request Apr 7, 2025
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.

--make-paths-enum renames the paths URL

7 participants