fix: Add tests for previous reactive hint fix, with more fixes. #1080
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stephenh
commented
May 9, 2024
| field.otherMetadata().allFields[field.otherFieldName] ?? | ||
| fail(`No field ${field.otherMetadata().type}.${field.otherFieldName}`); | ||
| const otherFieldName = | ||
| otherField.kind === "poly" ? `${field.otherFieldName}@${meta.type}` : field.otherFieldName; |
There was a problem hiding this comment.
I extract this to maybeAddTypeFilterSuffix so it could be used across the relation cases
This adds a test to reproduce the prior "walk through a different subtype that does not have the relation" error, as well as applies the fix to m2o and m2m handling as well. There is also a related fix that runtime filtering of the type filter, i.e. `publisher@Publisher` should pass for subtypes of `Publisher` like either a `LargePublisher` or `SmallPublisher`, and previously we were only exact-matching on the type name.
stephenh
force-pushed
the
fix-tests-for-cte-filtering
branch
from
5c166ce
to
16898a7
Compare
stephenh
commented
May 9, 2024
| expect(reverseReactiveHint(Author, Author, "firstName")).toEqual([ | ||
| { entity: Author, fields: ["firstName"], path: [] }, | ||
| ]); | ||
| expect(reverse(Author, Author, "firstName")).toEqual([{ entity: "Author", fields: ["firstName"], path: [] }]); |
There was a problem hiding this comment.
I added a helper method to turn entity into a string name, to stop blowing up Jest diffs on the entity constructors.
stephenh
commented
May 9, 2024
| @@ -264,11 +288,11 @@ export async function followReverseHint(entities: Entity[], reverseHint: string[ | |||
| // And "walk backwards" through the reverse hint | |||
| while (paths.length) { | |||
| const path = paths.shift()!; | |||
| const [fieldName, viaPolyType] = path.split("@"); | |||
| const [fieldName, viaType] = path.split("@"); | |||
There was a problem hiding this comment.
Renamed to just viaType since it's used for both polys & CTE/inheritance now
stephenh
commented
May 9, 2024
| if (viaPolyType) { | ||
| const loaded: Entity = await loadPromise; | ||
| return loaded && getMetadata(loaded).type === viaPolyType ? loaded : undefined; | ||
| function maybeApplyTypeFilter(loadPromise: Promise<Entity | Entity[]>, viaType: string | undefined) { |
There was a problem hiding this comment.
Took out the async/await
stephenh
commented
May 9, 2024
| if (meta.type === typeName) return true; | ||
| // Otherwise see if the entity is a subtype of the typeName, i.e. if our poly/type | ||
| // filter is `@Publisher`, and we're a `SmallPublisher`, that's valid to traverse. | ||
| for (const other of getSubMetas(meta)) { |
There was a problem hiding this comment.
Now we allow subtypes of the filter to pass
stephenh
pushed a commit
that referenced
this pull request
May 9, 2024
## [1.164.3](v1.164.2...v1.164.3) (2024-05-09) ### Bug Fixes * Add tests for previous reactive hint fix, with more fixes. ([#1080](#1080)) ([3185237](3185237))
|
🎉 This PR is included in version 1.164.3 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds a test to reproduce the prior "walk through a different subtype that does not have the relation" error, as well as applies the fix to m2o and m2m handling as well.
There is also a related fix that runtime filtering of the type filter, i.e.
publisher@Publishershould pass for subtypes ofPublisherlike either aLargePublisherorSmallPublisher, and previously we were only exact-matching on the type name.