Update to version 3.4 throw ex :[Right-hand side of 'instanceof' is not an object]

[codesculpture]

Duplicating this issue since it is closed, but it should be opened.

This is a interesting issue, since a single change did lot of things from this single commit

Object.hasOwn(value, 'getRootNode') is replaced by 'getRootNode' in value

But which is semantically not equal, as per mdn,

Object.hasOwn

The Object.hasOwn() static method returns true if the specified object has the indicated property as its own property. If the property is inherited, or does not exist, the method returns false.

in operator

The in operator tests if a string or symbol property is present in an object or its prototype chain. If you want to check for only non-inherited properties, use Object.hasOwn() instead.

That being said, if obj has prop and prop is a inherited member,

Object.has(obj, prop) would return false, which is correct
whereas,
'prop' in obj would return true, which is also correct as per spec.

So what happened here is that getRootNode is from Node
But what we have as value is HTMLElement
which inherits Node,

so Object.hasOwn(value, 'getRootNode') probably returns false, since getRootNode is a inherited member from Node in HTMLElement
but 'getRootNode' in value would happily returns true, since that is correct.
So the logic previously returned false but now the same logic returning true, which changed the execution flow and would execute this block of code
https://github.com/1904labs/dom-to-image-more/blob/e7d6ab0535a2477eb1a2b7226105fa1b7c6075a1/src/dom-to-image-more.js#L648C1-L655C10

where global would be a empty object, if dom-to-image-more was loaded through dynamic import using webpack, where webpack is importing dom-to-image-more by eval(import("https://mycdn.com/dom-to-image-more.min.js")) , here if a script is executed through eval the this object would be a empty object {}
For example,
eval(this) would returns {}

so global is {} at this point, so we are doing value instanceof global.ShadowRoot where global is {} thus the rhs of instanceof would be undefined. Throwing error.

The error occured for me while using bundler such as webpack and dynamically imported dom-to-image-more using import().

I believe the fix would be

 function getWindow(node) {
            const ownerDocument = node ? node.ownerDocument : undefined;
            return (
                (ownerDocument ? ownerDocument.defaultView : undefined) ||   
                window || 
                global
            );
        }

i.e the the existence of window would be checked before global (just swapping two lines)

@IDisposable

[IDisposable]

Thanks for the extensive write-up. Sadly the code you are referencing is not in the current release (neither the hasOwn, nor hasOwnProperty, nor in)

        function isInShadowRoot(value) {
            // not calling the method, getting the method
            if (value === null || value === undefined || value.getRootNode === undefined) return false;
            return isShadowRoot(value.getRootNode());
        }

While I understand that the referenced commit broke your use-case, we still need to have the shadowed elements code work. Since I still don't have a "this doesn't work" reproduction, I can't actually fix this error.

[IDisposable]

I will try swapping those two lines and see if anything breaks, but I don't have any test coverage for use in any browsers besides Chrome and no tests for use in Node or package-managed uses... so it may break things elsewhere.

[IDisposable]

There's a prerelease v.3.4.5-beta.0, please try it @codesculpture

[codesculpture]

Thanks for the extensive write-up. Sadly the code you are referencing is not in the current release (neither the hasOwn, nor hasOwnProperty, nor in)

        function isInShadowRoot(value) {
            // not calling the method, getting the method
            if (value === null || value === undefined || value.getRootNode === undefined) return false;
            return isShadowRoot(value.getRootNode());
        }

This is equivalent to in, in != Object.hasOwn, so this change is also not equivalent to Object.hasOwn

Hence, the logic is changed anyway.

[IDisposable]

If you can give a PR that adds a test so I can verify the bug and the fix, then I can address the situation. Otherwise, the best I can suggest is using an earlier version. The ShadowDOM fix is not getting backed out for something I cannot reproduce.

Have you tested the 3.4.5-beta.0 release?

[codesculpture]

Seems 3.4.5-beta.0 fixed the issue, this commit

To reproduce this issue,
create a react app using npx create-vite, and choose React, Javascript.
npm i [email protected]

And in App.jsx

const transform = async (dom) => {
  const domtoimage = await import('dom-to-image-more');
  await domtoimage.toSvg(dom, {});
}
function App() {
  return (
    <div> 
    <div id="hello"> </div>
     <button onClick={() => {
      transform(document.getElementById("hello"))
    }}> Hello </button>
    </div>
  )
}
export default App

Click Hello button.

Expected

No errors

Actual

Uncaught (in promise) TypeError: Right-hand side of 'instanceof' is not an object
    at n2 (dom-to-image-more.js:662:20)
    at o2 (dom-to-image-more.js:668:20)
    at Object.isShadowSlotElement (dom-to-image-more.js:709:17)
    at t4 (dom-to-image-more.js:443:26)
    at c2 (dom-to-image-more.js:407:38)
    at dom-to-image-more.js:371:24
    at async transform (App.jsx:3:3)

But if you switch to 3.4.5-beta.0 by
npm i [email protected]

Then if you click Hello button,
you no longer see the error.

Seems bundlers (i tested with rollup and webpack), bundles dynamic import by passing custom this object (through .call(obj, ...)) which might not be a window object, then global is what this is. So the fix seems okay which gives high priority for window over global (i.e window || global instead of global || window)

@IDisposable Am not sure is it possible to write test case for this scenario, wdyt

[IDisposable]

Glad that it fixed it... we should just hope it broke nothing new... all my existing tests still work.

Released as v3.4.5 (non-beta)

[codesculpture]

Glad that it fixed it... we should just hope it broke nothing new... all my existing tests still work.

Released as v3.4.5 (non-beta)

If existing tests works, then i guess we are good. Will try to write test that it imports dom-to-image-more using eval since thats how bundlers were importing.

Also thanks @IDisposable