Repository navigation
fix(core): evaluate nested exists false filter when parent key is nullish (#5017) - #5023
GauravPatil2515 wants to merge 1 commit into
Conversation
…lish (triggerdotdev#5017) Signed-off-by: Gaurav Patil <gauravpatil2516@gmail.com>
|
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hi @GauravPatil2515, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const safePayload = payload ?? {}; | ||
|
|
||
| for (const [patternKey, patternValue] of Object.entries(filter)) { | ||
| const payloadValue = payload[patternKey]; | ||
| const payloadValue = safePayload[patternKey]; |
There was a problem hiding this comment.
🟡 Missing nested keys appear to exist
For a nullish parent, safePayload makes $exists: false fail on keys like constructor. The fallback {} inherits those keys, so a missing child appears present.
Learn more
The matcher treats nullish payloads as an empty object so nested filters can evaluate missing children. Ordinary {} inherits keys such as constructor, toString, and __proto__. Reading one of those keys returns an inherited value rather than undefined, which makes $exists: false reject an absent child. The same lookup also affects a parent supplied as {}, but that case predates this change.
Example: With payload { user: null } and filter { user: { constructor: [{ $exists: false }] } }, the matcher returns false. No constructor child exists, so this filter must match.
Recommended fix: Resolve payloadValue using an own-property check before indexing safePayload, returning undefined for inherited keys. Cover nullish parents and ordinary empty objects with a regression test.
Was this helpful? React with 👍 or 👎 to provide feedback.
| return false; | ||
| } | ||
| } | ||
| const safePayload = payload ?? {}; |
There was a problem hiding this comment.
🔍 User-facing filter fix lacks a changeset
This changes published filter behavior, but the PR has no changeset. Repository guidance calls for one for user-facing package fixes.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Description
Resolves #5017.
In
packages/core/src/eventFilterMatches.ts, an early-return check previously evaluatedpayload === undefined || payload === nullby immediately returningfalsefor any non-empty filter. Consequently, nested negative checks such as{ user: { email: [{ $exists: false }] } }evaluated tofalsewhenuserwas absent ornullin the payload, even thoughuser.emailcorrectly does not exist in that case.This PR replaces the early bailout with
const safePayload = payload ?? {}, allowing nested filter evaluation to naturally traverse nullish ancestor objects and correctly resolve$exists: false.Changes Made
eventFilterMatchesinpackages/core/src/eventFilterMatches.tsto evaluate keys against a fallback safe payload rather than discarding valid$exists: falseleaves.packages/core/test/eventFilterMatches.test.tscovering nested$exists: falsewhen parent keys are missing or null.Checklist