perf: Replace deepcopy with Node-native structuredClone for faster object-cloning throughout the codebase
#9964
Conversation
- Removed the deepcopy dependency from the project. - Updated instances in the codebase to use structuredClone for deep copying objects, enhancing performance and reducing package size. - Cleaned up package.json and package-lock.json by removing the deepcopy entry.
|
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughThe pull request removes the external Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes While affecting 13 files, the changes follow a consistent, repetitive pattern—direct substitution of
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
chderen
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/Controllers/SchemaController.js (2)
570-587: CLP cloning via structuredClone preserves isolation from cached schema objectsSwitching to
structuredClone(schema.classLevelPermissions)here keepsSchemaData’s per‑class CLP copy isolated from the underlyingschemainstance, which is important because the following loop mutatesdata.classLevelPermissions.protectedFields[...]. For the typical CLP shape (nested plain objects/arrays of strings),structuredCloneis an appropriate drop‑in replacement fordeepcopy.If there is any path where
schema.classLevelPermissionscan beundefinedornullwhilethis.__protectedFields[schema.className]is set, you may want to defensively default to an empty object to avoid potentialTypeErrorwhen accessing.protectedFields:- data.classLevelPermissions = structuredClone(schema.classLevelPermissions); + data.classLevelPermissions = structuredClone(schema.classLevelPermissions || {});Please verify against your existing CLP/protected‑fields tests before adopting this change.
758-772: No-op.then(() => { })on reloadDataPromise can be removedThe final
.then(() => { });inreloadDatadoesn’t change behavior—the previous.then(...)already resolves toundefinedand propagates errors. It just adds an extra microtask.You can simplify the chain by dropping the trailing
.then:- this.reloadDataPromise = this.getAllClasses(options) - .then( - allSchemas => { - this.schemaData = new SchemaData(allSchemas, this.protectedFields); - delete this.reloadDataPromise; - }, - err => { - this.schemaData = new SchemaData(); - delete this.reloadDataPromise; - throw err; - } - ) - .then(() => { }); + this.reloadDataPromise = this.getAllClasses(options).then( + allSchemas => { + this.schemaData = new SchemaData(allSchemas, this.protectedFields); + delete this.reloadDataPromise; + }, + err => { + this.schemaData = new SchemaData(); + delete this.reloadDataPromise; + throw err; + } + ); ```<!-- review_comment_end --> </blockquote></details> <details> <summary>src/Push/utils.js (1)</summary><blockquote> `129-135`: **Consider defaulting `where` before cloning in applyDeviceTokenExists** `applyDeviceTokenExists` still assumes `where` is an object; `structuredClone(where)` will propagate `undefined` (or other non-object values) as‑is, which then relies on all callers always passing a valid object. To make this more robust and self‑contained, you could preserve behavior for object inputs while safely handling missing `where`: ```diff -export function applyDeviceTokenExists(where) { - where = structuredClone(where); +export function applyDeviceTokenExists(where = {}) { + where = structuredClone(where); if (!Object.prototype.hasOwnProperty.call(where, 'deviceToken')) { where['deviceToken'] = { $exists: true }; } return where; }Please verify against existing Push specs to ensure no tests depend on throwing when
whereis absent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
package.json(0 hunks)src/Controllers/DatabaseController.js(3 hunks)src/Controllers/SchemaController.js(2 hunks)src/GraphQL/loaders/functionsMutations.js(1 hunks)src/GraphQL/loaders/parseClassMutations.js(3 hunks)src/GraphQL/loaders/parseClassQueries.js(2 hunks)src/GraphQL/loaders/schemaMutations.js(3 hunks)src/GraphQL/loaders/schemaQueries.js(1 hunks)src/GraphQL/loaders/usersMutations.js(3 hunks)src/LiveQuery/ParseLiveQueryServer.ts(7 hunks)src/Push/PushWorker.js(1 hunks)src/Push/utils.js(2 hunks)src/RestWrite.js(10 hunks)
💤 Files with no reviewable changes (1)
- package.json
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-04-30T19:31:35.344Z
Learnt from: RahulLanjewar93
Repo: parse-community/parse-server PR: 9744
File: spec/ParseLiveQuery.spec.js:0-0
Timestamp: 2025-04-30T19:31:35.344Z
Learning: In the Parse Server codebase, the functions in QueryTools.js are typically tested through end-to-end behavior tests rather than direct unit tests, even though the functions are exported from the module.
Applied to files:
src/LiveQuery/ParseLiveQueryServer.tssrc/GraphQL/loaders/schemaMutations.jssrc/GraphQL/loaders/schemaQueries.js
📚 Learning: 2025-12-02T08:00:20.138Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-12-02T08:00:20.138Z
Learning: For Parse Server 9 release (PR #9938 and related), the parse/push-adapter dependency must be upgraded to version >= 8.0.0, not 7.0.0. Version 8.x drops support for Node 18.
Applied to files:
src/Push/PushWorker.js
📚 Learning: 2025-08-26T14:06:31.853Z
Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: spec/CloudCode.spec.js:446-469
Timestamp: 2025-08-26T14:06:31.853Z
Learning: In the Parse Server codebase, when handling query objects in maybeRunAfterFindTrigger, objects without a where property that contain options like limit/skip should be treated as query JSON with an empty where clause using the spread pattern { where: {}, ...query }, not nested as { where: query }.
Applied to files:
src/GraphQL/loaders/parseClassQueries.jssrc/Push/utils.js
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: Tests in the parse-server repository should use promise-based approaches rather than callback patterns with `done()`. Use a pattern where a Promise is created that resolves when the event occurs, then await that promise.
Applied to files:
src/RestWrite.js
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`. The preferred pattern is to create a Promise that resolves when an expected event occurs, then await that Promise.
Applied to files:
src/RestWrite.js
🧬 Code graph analysis (10)
src/Push/PushWorker.js (2)
src/GraphQL/loaders/parseClassMutations.js (3)
structuredClone(77-77)structuredClone(180-180)structuredClone(286-286)src/GraphQL/loaders/parseClassQueries.js (1)
structuredClone(101-101)
src/GraphQL/loaders/parseClassQueries.js (1)
src/GraphQL/loaders/parseClassMutations.js (3)
structuredClone(77-77)structuredClone(180-180)structuredClone(286-286)
src/GraphQL/loaders/schemaMutations.js (2)
src/GraphQL/loaders/parseClassMutations.js (3)
structuredClone(77-77)structuredClone(180-180)structuredClone(286-286)src/GraphQL/loaders/parseClassQueries.js (3)
structuredClone(101-101)args(16-16)args(17-17)
src/GraphQL/loaders/schemaQueries.js (2)
src/GraphQL/loaders/parseClassMutations.js (3)
structuredClone(77-77)structuredClone(180-180)structuredClone(286-286)src/GraphQL/loaders/parseClassQueries.js (3)
structuredClone(101-101)args(16-16)args(17-17)
src/GraphQL/loaders/parseClassMutations.js (1)
src/GraphQL/loaders/parseClassQueries.js (3)
structuredClone(101-101)args(16-16)args(17-17)
src/GraphQL/loaders/usersMutations.js (2)
src/GraphQL/loaders/parseClassMutations.js (3)
structuredClone(77-77)structuredClone(180-180)structuredClone(286-286)src/GraphQL/loaders/parseClassQueries.js (3)
structuredClone(101-101)args(16-16)args(17-17)
src/Controllers/SchemaController.js (3)
src/RestWrite.js (1)
data(1733-1739)src/GraphQL/loaders/parseClassMutations.js (3)
structuredClone(77-77)structuredClone(180-180)structuredClone(286-286)src/GraphQL/loaders/parseClassQueries.js (1)
structuredClone(101-101)
src/Controllers/DatabaseController.js (2)
src/GraphQL/loaders/parseClassMutations.js (3)
structuredClone(77-77)structuredClone(180-180)structuredClone(286-286)src/GraphQL/loaders/parseClassQueries.js (1)
structuredClone(101-101)
src/Push/utils.js (3)
src/Push/PushWorker.js (2)
body(75-75)where(47-47)src/GraphQL/loaders/parseClassMutations.js (3)
structuredClone(77-77)structuredClone(180-180)structuredClone(286-286)src/GraphQL/loaders/parseClassQueries.js (1)
structuredClone(101-101)
src/GraphQL/loaders/functionsMutations.js (2)
src/GraphQL/loaders/parseClassMutations.js (3)
structuredClone(77-77)structuredClone(180-180)structuredClone(286-286)src/GraphQL/loaders/parseClassQueries.js (3)
structuredClone(101-101)args(16-16)args(17-17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: PostgreSQL 15, PostGIS 3.4
- GitHub Check: PostgreSQL 18, PostGIS 3.6
- GitHub Check: PostgreSQL 15, PostGIS 3.5
- GitHub Check: MongoDB 7, ReplicaSet
- GitHub Check: Node 20
- GitHub Check: PostgreSQL 17, PostGIS 3.5
- GitHub Check: PostgreSQL 16, PostGIS 3.5
- GitHub Check: PostgreSQL 15, PostGIS 3.3
- GitHub Check: Redis Cache
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: Node 18
- GitHub Check: MongoDB 6, ReplicaSet
- GitHub Check: Node 22
- GitHub Check: Docker Build
- GitHub Check: Benchmarks
🔇 Additional comments (17)
src/GraphQL/loaders/functionsMutations.js (1)
44-48: structuredClone for GraphQL args is appropriate hereUsing
structuredClone(args)to extract{ functionName, params }keeps the GraphQL args immutable while allowing downstream mutation ofparams. Given GraphQL inputs are plain JSON, this is a safe replacement fordeepcopy.Please confirm the runtime environment guarantees a global
structuredClone(or a polyfill), especially for older Node versions or custom runtimes.src/GraphQL/loaders/schemaQueries.js (1)
28-32: Consistent use of structuredClone for schema query argsCloning
argsbefore destructuringnamematches the pattern used in other GraphQL loaders and prevents accidental mutation of GraphQL’s argument object. No functional regression versusdeepcopyfor JSON-like inputs.Please double‑check that all Node/JS environments where this code runs expose
structuredCloneglobally (or have an appropriate polyfill wired up).src/GraphQL/loaders/schemaMutations.js (1)
28-32: Schema mutation handlers correctly migrated to structuredCloneAll three mutations (
CreateClass,UpdateClass,DeleteClass) now cloneargsviastructuredClonebefore destructuring, which keeps the original GraphQL args immutable while preserving previous deep‑copy semantics for plain JSON (names and schema field definitions).Since these mutations run on the server, please ensure your supported Node versions (and test runners) all provide
structuredCloneor a polyfill, to avoid runtimeReferenceErrorin older environments.Also applies to: 78-82, 130-133
src/GraphQL/loaders/parseClassMutations.js (1)
75-86: parseClass mutations: structuredClone preserves original GraphQL args while allowing mutationUsing
structuredClone(args)for{ fields },{ id, fields }, and{ id }is a good fit here: it keepsargs(andargs.fields) intact for use asoriginalFields, while allowingfieldsto be transformed/mutated downstream without side effects. This is in line with the rest of the GraphQL loaders.Please confirm your supported Node/JS environments and TS configuration (if any) guarantee availability of global
structuredClone, so these handlers don’t fail at runtime under older Node versions.Also applies to: 178-196, 284-293
src/Push/utils.js (1)
42-55: Locale transform now deep-clones push body with structuredCloneCloning
bodyviastructuredClone(body)before applying locale-specific overrides and stripping localized keys keeps the original push body immutable across locales, matching the previousdeepcopysemantics for JSON payloads.Please confirm all environments sending push notifications run on Node versions (or polyfills) that provide
structuredClone, since this function is now required for push processing.src/GraphQL/loaders/usersMutations.js (1)
32-43: User mutations: structuredClone is an appropriate drop-in for deep argument cloning
SignUp,LogInWith, andLogInnow cloneargsviastructuredClonebefore destructuring (fields,authData,username,password). For JSON‑like GraphQL inputs this is behaviorally equivalent to the previousdeepcopyuse and preserves the pattern whereargs.*serves as the untouched “original” while the local variables can be transformed.As with the other GraphQL loaders, please verify that all deployment and test environments support global
structuredClone(or include a compatible polyfill), so these mutations don’t break on older Node versions.Also applies to: 109-120, 173-185
src/LiveQuery/ParseLiveQueryServer.ts (2)
521-527: LiveQuery subscription matching: structuredClone is safe for JSON snapshotsUsing
structuredClone(parseObject)before callingmatchesQuerykeeps the JSON snapshot of the object immutable across matching operations, mirroring the intent of the previousdeepcopyusage. Because the callers pass intoJSON()results, the data is fully structured‑cloneable.Since this is a TypeScript file, please confirm your TS configuration includes a lib/target where
structuredCloneis available in the type system (or add an explicit typing/polyfill), and that all supported Node runtimes providestructuredCloneglobally.
255-260: Logging and error-message line breaks are purely cosmeticThe changes around
JSON.stringify(error)and the string concatenations in unsubscribe error messages only affect formatting/line breaks and not the actual log content or control flow. No functional impact here.Also applies to: 416-421, 821-825, 963-967, 993-1019
src/Push/PushWorker.js (1)
93-93: LGTM! Clean replacement with native API.The use of
structuredClonefor cloning the push notification body is appropriate here. Push payloads are plain JSON objects, making this a safe and performant replacement.src/Controllers/DatabaseController.js (2)
504-504: LGTM! Appropriate use of native cloning.Cloning the update object with
structuredCloneis the right approach here. Database update payloads are plain objects, making this replacement safe and more performant than the external library.
1545-1546: Good defensive programming improvement.The explicit
typeofcheck and use ofObject.prototype.hasOwnProperty.calladds safety when accessing thetypeproperty, preventing potential issues iffieldDescriptoris not an object or has unexpected prototype chains.src/GraphQL/loaders/parseClassQueries.js (1)
77-77: LGTM! Safe cloning of GraphQL resolver arguments.Using
structuredClonefor GraphQL resolver args is appropriate. The comment at line 100 correctly notes the purpose: preventing internal reassignment issues. GraphQL args are plain objects, making this a safe replacement.Also applies to: 101-101
src/RestWrite.js (5)
78-79: LGTM! Essential cloning to prevent mutation.Cloning
queryanddatain the constructor is critical to prevent unintended mutations. UsingstructuredCloneis appropriate here as these are plain REST API objects.
380-380: LGTM! Safe ACL object cloning.Using
structuredClonefor ACL objects is appropriate. ACL structures are plain objects with read/write permissions, making this a safe replacement.Also applies to: 383-383
602-602: LGTM! Preserves original user data for trigger.Cloning the user result before passing to the beforeLogin trigger is good practice, ensuring the trigger receives an isolated copy and cannot mutate the original data.
1739-1739: LGTM! Appropriate cloning in data transformations.Using
structuredClonein the reduce callbacks forsanitizedDataandbuildParseObjectsis correct. This ensures the originalthis.dataremains unmodified during transformations.Also applies to: 1789-1789
78-79: No Node.js version compatibility issue withstructuredClone.The project's minimum supported Node.js version is 18.20.4 (as specified in
package.json), which is well above Node.js 17.0.0 wherestructuredClonewas introduced. The API is fully supported across all configured version ranges.Likely an incorrect or invalid review comment.
|
The CI fails consistently for these tests: |
mtrezza
changed the title
deepcopy with Node-native structuredClone for faster deep-copy operations
mtrezza
changed the title
deepcopy with Node-native structuredClone for faster deep-copy operationsdeepcopy with Node-native structuredClone for faster object-cloning throughout the codebase
mtrezza
changed the title
deepcopy with Node-native structuredClone for faster object-cloning throughout the codebasedeepcopy with Node-native structuredClone for faster object cloning throughout the codebase
mtrezza
changed the title
deepcopy with Node-native structuredClone for faster object cloning throughout the codebasedeepcopy with Node-native structuredClone for faster object-cloning throughout the codebase
|
Since this is a perf improvement, this PR should add a perf benchmark as well. The current benchmark shows no significant improvement or degradation, which is a good sign so far. A dedicated test that shows improvement would be good, but if the perf change is insignificant that's also acceptable as we prefer native APIs over ext. dependencies. |
4 participants
🔄 Replace
deepcopywith NativestructuredClone✔️ Summary
This PR replaces all usages of the
deepcopypackage with the nativestructuredCloneAPI.It also removes the
deepcopydependency frompackage.jsonandpackage-lock.json.🎯 Motivation
structuredCloneis now widely supported in Node.js and provides faster deep-copy operations.🔍 Changes Included
deepcopy(...)calls withstructuredClone(...)deepcopyfrom dependency lists🧪 What to Review
(especially nested objects, arrays, and edge cases).
deepcopy.Summary by CodeRabbit
deepcopypackage dependency and replaced all cloning operations with the native JavaScriptstructuredClonemethod across the application, reducing external dependencies while maintaining equivalent functionality.✏️ Tip: You can customize this high-level summary in your review settings.