Remove enableRegex #3097
Remove enableRegex #3097
Conversation
🦋 Changeset detectedLatest commit: 1223f8a The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
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 |
|
You have successfully added a new CodeQL configuration |
Performance ReportNo Performance Changes Show Full Table
Old Schema Generation: 28.692s |
| const matchesSetting: boolean | undefined = features?.filters?.[f.typeMeta.name]?.MATCHES; | ||
| if (matchesSetting !== undefined) { | ||
| if (matchesSetting === true) { | ||
| res[`${f.fieldName}_MATCHES`] = { type: "String", directives: deprecatedDirectives }; | ||
| } | ||
| } else if (enableRegex) { | ||
| // TODO: This is deprecated. To be removed in 4.0.0. | ||
| res[`${f.fieldName}_MATCHES`] = { type: "String", directives: deprecatedDirectives }; | ||
| } |
Co-authored-by: Darrell Warde <[email protected]>
angrykoala
left a comment
angrykoala
left a comment
There was a problem hiding this comment.
Looks good, just one important comment that we should either tackle or discuss before merging
| } | ||
|
|
||
| const stringWhereOperators = ["_CONTAINS", "_STARTS_WITH", "_ENDS_WITH"]; | ||
| const stringWhereOperators: { comparator: string; typeName: string }[] = [ |
There was a problem hiding this comment.
nitpick, not important: For these complex object I find array types to be a bit more readable as
Array<{ comparator: string; typeName: string }>| if (filter === "MATCHES") { | ||
| return; | ||
| } | ||
| Object.entries(features?.filters?.[f.typeMeta.name] || {}).forEach(([filter, enabled]) => { |
There was a problem hiding this comment.
nitpick Consider creating a varaible for f.typeMeta.name as it is used in several places and the extra context of a explicit name could help with readability here
| if (filter === "MATCHES") { | ||
| return; | ||
| } | ||
| Object.entries(features?.filters?.[f.typeMeta.name] || {}).forEach(([filter, enabled]) => { |
There was a problem hiding this comment.
Also, I think we could really make use of a features helper here, as this seems like it is going to be the way forward for feature flags and we should make our lifes a bit easier on this.
I'm thinking something a bit like:
features(f.typeMeta.name).hasFilter("MATCHES");This way we have a more unified way of accesing features that don't require concatenation of .? and default objects and we could encapsulate the logic of traversing the filters, avoiding this forEach on our schema generation logic.
Thoughts?
4 participants
Description
This PR removes
config.enableRegexas it has been replaced byMATCHESinfeatures.filters.Complexity
Low