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

Add option for omitting custom fields #23

Merged
merged 4 commits into from
Mar 11, 2021
Merged

Conversation

x1frm
Copy link
Contributor

@x1frm x1frm commented Feb 6, 2021

#19

lib/index.test.ts Outdated Show resolved Hide resolved
lib/index.ts Outdated Show resolved Hide resolved
lib/index.ts Outdated
} = options;
props = [...defaultSupportedMetaProps, ...props];
omitted = new Set(options.omitId ? ['__v', '_id'] : ['__v']);
omitted = new Set(omitId ? ['__v', '_id'] : ['__v']);
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a reason you're creating a new set instead of just adding the keys to the existing set?

Copy link
Contributor Author

@x1frm x1frm Feb 16, 2021

Choose a reason for hiding this comment

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

Yes.
omitted variable is in the outer scope, so if we change it, the new value will be used in next function calls.
By making a new set we drop it back to default every function call.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah good catch -

the omitted variable in the root scope is in use by the function removeOmitted also in the root scope.

assigning the new Set to the root level omitted variable still globally mutates the root scope.

since removeOmitted is only used once within the scope of documentModel you could just move that function declaration to be in scope of documentModel and set it once as a fn of defaults + input omitFields

const removeOmitted = (swaggerFieldSchema: {

return swaggerFieldSchema.type != null && !omitted.has(swaggerFieldSchema.field);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@blugavere blugavere left a comment

Choose a reason for hiding this comment

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

please add documentation for the new feature to the readme.

@x1frm x1frm changed the title add omit '_id' option Add option for omitting custom fields Feb 16, 2021
@x1frm x1frm requested a review from blugavere February 18, 2021 09:35
@blugavere blugavere merged commit 2afc0c5 into giddyinc:master Mar 11, 2021
@blugavere
Copy link
Contributor

published @ [email protected]

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.

2 participants