Skip to content

[FIX] sap/base/util/deepEqual: guard browser-global Node reference#4451

Open
martindglaser wants to merge 1 commit into
UI5:masterfrom
martindglaser:fix/deepequal-avoid-node-global
Open

[FIX] sap/base/util/deepEqual: guard browser-global Node reference#4451
martindglaser wants to merge 1 commit into
UI5:masterfrom
martindglaser:fix/deepequal-avoid-node-global

Conversation

@martindglaser

Copy link
Copy Markdown
Contributor

Summary

sap/base/util/deepEqual compares DOM nodes via a instanceof Node. Node is a browser global that does not exist in non-browser environments such as Web Workers or Node.js, so deepEqual() throws a ReferenceError: Node is not defined there when comparing any two objects — even though it is part of the environment-agnostic sap/base package. The module's own @evo-todo already flags this ("dependency to global name 'Node' contradicts sap/base package"). This complements the recent removal of window usages from sap/base (6601e5b), which did not cover Node.

Fix

Guard the reference with typeof Node !== "undefined" so DOM nodes are still compared in the browser while deepEqual stays usable in environments without a Node global. No behavior change in the browser (public API unchanged).

How to reproduce

In a Web Worker or Node.js context:

deepEqual({ a: 1 }, { a: 1 }); // ReferenceError: Node is not defined

deepEqual() compared DOM nodes via `a instanceof Node`. `Node` is a browser
global that does not exist in non-browser environments such as Web Workers or
Node.js, so deepEqual() threw a ReferenceError there when comparing any two
objects - even though it belongs to the environment-agnostic sap/base package
(as the existing @evo-todo already notes).

Guard the reference with a typeof check so DOM nodes are still compared in the
browser while deepEqual stays usable in environments without a 'Node' global.
@kgogov

kgogov commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Hello @martindglaser,

Thank you for creating this PR — much appreciated! 🙌

I've created internal incident DINC0976821 to track this. Colleagues responsible for this module will look it up soon.

Regards,
Konstantin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants