Skip to content
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

fix: strictly check dependencies import judgment using regular expressions #1146

Conversation

soartec-lab
Copy link
Member

@soartec-lab soartec-lab commented Jan 6, 2024

Status

READY

Description

fix #1145

The current judgment logic uses a simple condition of whether the string exists.
Therefore, for example, if the variable useKey is used, Key will be imported, but this may be an unnecessary import.

I achieved this by using regular expressions to determine.

Related PRs

none

Todos

  • Tests
  • Documentation
  • Changelog Entry (unreleased)

Steps to Test or Reproduce

  1. prepare OpenAPI definition for pet store
  2. execute orval
orval
  1. We have confirmed that the existing generation process is not destroyed using the pet store.

@soartec-lab soartec-lab changed the title feat: strictly check dependencies import judgment using regular expressions fix: strictly check dependencies import judgment using regular expressions Jan 6, 2024
@soartec-lab soartec-lab force-pushed the feat/strictly-check-dependencies-import branch from 768fb67 to 9f349c4 Compare January 6, 2024 02:36
implementation.includes(e.alias || e.name),
);
const toAdds = exports.filter((e) => {
const petternValues = [e.alias, e.name].filter((p) => p?.length).join('|');
Copy link
Collaborator

Choose a reason for hiding this comment

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

PatternValues not PetternValues

Copy link
Member Author

@soartec-lab soartec-lab Jan 6, 2024

Choose a reason for hiding this comment

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

@melloware
Thank you for review.
I interpreted the name of the variable to be inappropriate. Do you think this is searchWords in the sense of the words you want to search for in a regular expression?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes searchWords would be a good name

Copy link
Member Author

Choose a reason for hiding this comment

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

@soartec-lab soartec-lab force-pushed the feat/strictly-check-dependencies-import branch from 9f349c4 to 5d01571 Compare January 6, 2024 03:06
@soartec-lab soartec-lab requested a review from melloware January 6, 2024 03:08
@melloware melloware merged commit c42fcda into orval-labs:master Jan 6, 2024
2 checks passed
@soartec-lab soartec-lab added the bug Something isn't working label Jan 28, 2024
@soartec-lab soartec-lab added this to the 6.24.0 milestone Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Makes the decision process for importing dependencies stricter
2 participants