fix(delegate): make sure concrete fields are included when a polymorphic model field is included in deep nesting #1524
Conversation
…hic model field is included in deep nesting Fixes #1522
WalkthroughWalkthroughRecent changes enhance the Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (3)
packages/runtime/src/enhancements/delegate.ts (3)
Line range hint
286-292: Theelseclause is unnecessary as the previous branches containbreakstatements, simplifying the control flow.- } else { - target = thisLayer; - base = this.getBaseModel(base.name); - }
Line range hint
307-307: Avoid using thedeleteoperator for performance reasons. Consider setting the property toundefinedinstead.- delete where[field]; + where[field] = undefined;
Line range hint
960-960: Consider using optional chaining for better readability and to avoid potential runtime errors.- if (subModelName) { + if (subModelName?) {
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/runtime/src/enhancements/delegate.ts (1 hunks)
- tests/regression/tests/issue-1522.test.ts (1 hunks)
Additional context used
Biome
packages/runtime/src/enhancements/delegate.ts
[error] 92-94: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 132-135: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 170-173: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 286-292: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 307-307: Avoid the delete operator which can impact performance. (lint/performance/noDelete)
Unsafe fix: Use an undefined assignment instead.
[error] 330-330: Avoid the delete operator which can impact performance. (lint/performance/noDelete)
Unsafe fix: Use an undefined assignment instead.
[error] 653-684: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 717-717: Avoid the delete operator which can impact performance. (lint/performance/noDelete)
Unsafe fix: Use an undefined assignment instead.
[error] 780-780: Avoid the delete operator which can impact performance. (lint/performance/noDelete)
Unsafe fix: Use an undefined assignment instead.
[error] 788-788: Avoid the delete operator which can impact performance. (lint/performance/noDelete)
Unsafe fix: Use an undefined assignment instead.
[error] 960-960: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
Additional comments not posted (1)
tests/regression/tests/issue-1522.test.ts (1)
1-1: Ensure theloadSchemaimport is utilized correctly in the script.
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (3)
packages/runtime/src/enhancements/delegate.ts (3)
Line range hint
92-94: Consider removing unnecessary else clauses.- else { - target = thisLayer; - base = this.getBaseModel(base.name); - }The else clause can be omitted because the preceding branches end with a
breakorreturnstatement, making the else unnecessary and potentially confusing.Also applies to: 132-135, 170-173, 286-292, 653-684
Line range hint
307-307: Consider replacing thedeleteoperator with an assignment toundefined.- delete args[kind][field]; + args[kind][field] = undefined;Using
deletecan lead to performance issues due to changes in the underlying data structure of objects. Assigningundefinedis generally more performant and should be preferred unless you need to remove a property completely to free up memory.Also applies to: 330-330, 717-717, 780-780, 788-788
Line range hint
960-960: Use optional chaining to simplify the code.- if (subData && typeof subData === 'object') { + if (subData?.object) {This change leverages optional chaining to make the code cleaner and more readable by reducing unnecessary checks.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/runtime/src/enhancements/delegate.ts (1 hunks)
Additional context used
Biome
packages/runtime/src/enhancements/delegate.ts
[error] 92-94: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 132-135: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 170-173: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 286-292: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 307-307: Avoid the delete operator which can impact performance. (lint/performance/noDelete)
Unsafe fix: Use an undefined assignment instead.
[error] 330-330: Avoid the delete operator which can impact performance. (lint/performance/noDelete)
Unsafe fix: Use an undefined assignment instead.
[error] 653-684: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 717-717: Avoid the delete operator which can impact performance. (lint/performance/noDelete)
Unsafe fix: Use an undefined assignment instead.
[error] 780-780: Avoid the delete operator which can impact performance. (lint/performance/noDelete)
Unsafe fix: Use an undefined assignment instead.
[error] 788-788: Avoid the delete operator which can impact performance. (lint/performance/noDelete)
Unsafe fix: Use an undefined assignment instead.
[error] 960-960: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
Fixes #1522