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(core): filters = {} treated as undefined #1740

Closed
wants to merge 1 commit into from

Conversation

melloware
Copy link
Collaborator

Fix #1736 fix(core): filters = {} treated as undefined

@redsky17
Copy link
Contributor

I hope you don't mind a random person chiming in on this pull request, but I wanted to mention something I discovered today in debugging a regression between 7.2.0 and 7.3.0 on my own code base. The commit that introduced the new 'exclude' and 'include' functionality for filters (#1690) changed the logic that gets applied in the generateSchemasDefinition in schema-definition.ts to determine if the filtering should run. Previously, if filters.schemas was undefined, the if (schemasFilters) would evaluate to false and the block would not run. Now, if you have a filters but filters.schemas is undefined if (filters) evaluates to true and the block is executed with an empty array as the schemasFilters value. This causes schemas to get filtered out and results in schema code with missing definitions.

@melloware
Copy link
Collaborator Author

Oh nice. I don't mind at all. Did you want to submit a PR?

@soartec-lab
Copy link
Member

soartec-lab commented Dec 11, 2024

I’ll wait for this discussion for a short time 👀

@redsky17
Copy link
Contributor

redsky17 commented Dec 11, 2024

I could certainly open a PR if you would like, but if you would like to take care of it yourself that works for me! I'm not exactly sure what coding style is typically employed on your codebase (the debugging I was doing on my end was the as-built npm package 'dist' files, so they were esbuild-ified). I think I would simply change the if check to if(filters?.schemas) and remove the || [] part from the subsequent line (or, perhaps, remove the schemasFilters array object entirely from the code). I guess let me know what you think the most appropriate approach is, and I can write up the change and update any tests as well (again, if you want me to :) ).

@melloware
Copy link
Collaborator Author

@redsky17 that sounds right feel free to submit a PR I think your fix is the right one.

@redsky17
Copy link
Contributor

I have opened #1744. Thanks folks!

@melloware melloware closed this Dec 12, 2024
@melloware melloware deleted the 1736 branch December 12, 2024 18:15
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
3 participants