Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Version 3.1.2 breaks our code with TypeError: Cannot read property 'Node' of undefined #1252

Closed
lpatiny opened this issue Jan 28, 2022 · 8 comments

Comments

@lpatiny
Copy link

lpatiny commented Jan 28, 2022

Bug report

Explanation

Since the last release of (3.1.2) the test cases in our open-source project are not passing anymore:

https://github.com/cheminfo/mass-tools/runs/4983444371?check_suite_focus=true#step:6:462

The code that generates this error is quite complex and if this error is not obvious I may try to reduce the code and create a fiddle.

In the meantime I forced versions 3.1.1 that works perfectly.

@Fuzzyma
Copy link
Member

Fuzzyma commented Jan 28, 2022

The failing test is caused by a change in nodeOrNew:

export function nodeOrNew (name, node) {
  return (node && node.ownerDocument && node instanceof node.ownerDocument.defaultView.Node) ? node : create(name)
}

before it was checking for instance of window.Node which caused the problem that it wouldn't work when reaching into object tags (because they have their own version of Node).

It seems like your framework that you are using for testing does not support defaultView because that yields undefined in your test and makes it fail

@lpatiny
Copy link
Author

lpatiny commented Jan 29, 2022

Because we are generating the SVG either on the server or on the client we are using svgdom library.

https://github.com/cheminfo/mass-tools/blob/9439a46477bb234eaa2c81facc539c0ec52f004f/packages/ms-report/src/getPaper.js#L3-L8

Do you see a simple way to let it work again with svgdom ?

@Fuzzyma
Copy link
Member

Fuzzyma commented Jan 29, 2022

I am using svgdom for testing as well and it's all green 🤔

@Fuzzyma
Copy link
Member

Fuzzyma commented Jan 29, 2022

Oh, you are running a rather old version of svgdom. Updating should solve this problem

@Zennii
Copy link

Zennii commented Feb 12, 2022

I'm also having an issue with this nodeOrNew function, in Firefox 97.0:

I'm parsing some XML into an SVG in a pretty simple way:

  let newBg = SVG(xhttp.responseXML);
  let newLen = newBg.find('path').length;

And then I get this error stemming from .find('path'):

Uncaught TypeError: e.ownerDocument.defaultView is null
    E adopter.js:44
    Path Path.js:10
    j adopter.js:72
    SVG selector.js:8
    s utils.js:8
    pt selector.js:7
    find selector.js:14
    onreadystatechange beta.main.js:340
    EventHandlerNonNull* beta.main.js:325

As far as I'm aware, it was working within the last 1 or 2 days and now it's not... Going back to 3.1.1 is my solution for now

@mradcliffe
Copy link
Contributor

I also ran into this when upgrading 3.0.16 to 3.1.2, but only after I updated TypeScript 4.3.5 to 4.6.4.

Previously I needed to get svg files via XHR/fetch and then I'd parse into an SVGElement (DOM Element type in TypeScript). Then I'd created the SVG element via SVG<SVGElement>(part); and use addTo method to attach it to a container. At first I ran into an issue with document fragments (jQuery()), but then after I changed to using DOMParser(), I got this error.

To resolve it, I needed to call document.importNode:

map((data: string): SVGElement => {
  const doc: XMLDocument = new DOMParser().parseFromString(data, 'image/svg+xml');
  let svgEl: SVGElement;
  if (!doc.querySelector('parsererror')) {
    // Note: this could have some performance impacts if document is not cleaned up.
    svgEl = document.importNode(doc.firstElementChild, true) as SVGElement;
  }
  return svgEl;
),

Then essentially

const imported = SVG<SVGElement>(svgEl);
const el = imported.addTo(someWrapperElement);

Hope this helps.

@Fuzzyma Fuzzyma closed this as completed in 9ed85cf Sep 3, 2023
@Fuzzyma
Copy link
Member

Fuzzyma commented Sep 3, 2023

I relaxed the code so that it allows unimported nodes as well.
However, you have to be careful here. I dont know how browser handle not imported nodes but svgdom will not import nodes for you automatically which might lead to other bugs down the road.

You are better off just using the SVG() method to create svg from svg-strings:

const rect = SVG('<rect width="100" height="100" />')

This will take care of that problem for you

@Fuzzyma
Copy link
Member

Fuzzyma commented Jun 18, 2024

I am sorry, I somehow forgot to publish all the fixes I made. I just released it:
https://github.com/svgdotjs/svg.js/releases/tag/3.2.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants