Implement SVG support

[EvHaus]

When using .find() on components with an <svg/> tag, enzyme explodes with:

TypeError: classes.replace is not a function
    at instHasClassName (webpack:///./~/enzyme/build/MountedTraversal.js?:69:21)
    at eval (webpack:///./~/enzyme/build/MountedTraversal.js?:224:20)
    at findAllInRenderedTreeInternal (webpack:///./~/enzyme/build/MountedTraversal.js?:282:13)
    at findAllInRenderedTreeInternal (webpack:///./~/enzyme/build/MountedTraversal.js?:294:24)
    at findAllInRenderedTreeInternal (webpack:///./~/enzyme/build/MountedTraversal.js?:279:12)
    at treeFilter (webpack:///./~/enzyme/build/MountedTraversal.js?:307:10)
    at ReactWrapper.eval (webpack:///./~/enzyme/build/ReactWrapper.js?:62:12)
    at eval (webpack:///./~/enzyme/build/ReactWrapper.js?:1166:21)
    at Array.map (native)
    at ReactWrapper.flatMap (webpack:///./~/enzyme/build/ReactWrapper.js?:1165:32)

Here is a very short and simple test case to reproduce the issue:

import {mount} from 'enzyme';
const component = mount(<svg />);
component.find('.test');

The problem is that SVG elements do not report their className as a string, but as an object instead.

[aweary]

@EvNaverniouk what version of React are you using? I'm unable to reproduce on the current master branch with any recent major version of React (13, 14, 15).

    it('should work with svgs', () => {
      const wrapper = mount(<svg />);
      console.log(wrapper.find('.test'));
    });

[EvHaus]

I'm on React 15.0.2, Enzyme 2.3.0, JSDom 9.0.0. I'm also using the css-modules-require-hook module, but disabling it doesn't appear to fix the issue either.

[aweary]

Can you share the exact code that is failing for you? This should definitely be failing for me as far as I can tell https://jsfiddle.net/1pxpbp0n/, so I'll have to look into it more.

edit: it looks like we're using JSDom 6.x for development, I'm betting that returning objects for the SVG className attribute was introduced in a later version. I'll check on that.

edit2: Even with JSDom 9.0.0 finDOMNode(inst).className is returning a string for me. I'll try to create a test repo using 2.3.0 instead of the master branch to see if there's any difference.

[aweary]

@EvNaverniouk are you running this test in a real browser environment at all? Can I see your jsdom setup?

Using JSDom 9.0.0 I see the following behavior:

var svg = document.createElementNS('http://www.w3.org/2000/svg', 'svg');
console.log(svg.className)
// ''

Which is incorrect, it should return an object with the shape {baseVal: "", animVal: ""} so it looks like the reason it's passing for me is that JSDom isn't respecting createElementNS, so I'm curious whats happening on your end.

You can clone https://github.com/Aweary/enzyme-test-repo/tree/issue-%23375 and see that it's passing just fine. Feel free to fork that repo if you can provide a failing test case.

[EvHaus]

Using your JSFiddle link, I'm getting an object in the shape SVGAnimatedString {baseVal: "", animVal: ""}. I'm on Mac Google Chrome Version 50.0.2661.94 (64-bit). This is exactly the same object that's tripping up enzyme for me.

Working on a repo to reproduce the issue now. It seems like it's not as straightforward as I thought. Having trouble reproducing in a clean environment. As soon as I nail it down, I'll post the minimal case.

[aweary]

@EvNaverniouk right, the reason I asked whether you were testing in a browser environment is because I think jsdom is not handling document.createElementNS correctly so its not implementing the SVG interface. The JSFiddle outputs the SVGAnimatedString as I'd expect it to in Chrome. Try doing the following in your terminal using the Node REPL within the directory of your project:

var jsdom = require('jsdom').jsdom
global.document = jsdom('')
var svg = global.document.createElementNS('http://www.w3.org/2000/svg', 'svg');
console.log(svg.className)

Let me know if you're getting an object in jsdom as well, because I'm not and I think that's the issue.

[aweary]

@EvNaverniouk see jsdom/jsdom#1480 (comment). jsdom does not yet support the SVG DOM so this is not an issue with enzyme just yet. I think that we should likely add support for SVGAnimatedStrings preemptively anyways though so we can 1) support it when it does get implemented and 2) support any other DOM libraries that might actually have SVG DOM support.

[EvHaus]

Right. I see what you're saying.

Strangely, running my code through JSDom via the terminal is fine -- no errors. But if I run the same test code in Chrome without JSDom, I get the original issue above.

Ok. I'll close this and continue by doing a .find() on the component directly rather than relying on class name parsing.

[aweary]

@EvNaverniouk yeah that would be expected. Until jsdom supports SVGElements you wont be able to assert SVG-specific values.

[aweary]

I'm going to reopen this as I think we still want to consider implementing support for this so enzyme can work with SVGs in the browser.

[lelandrichardson]

It seems like the right approach here is to just have a getClassNameFromDOMNode function or something, which would just do .className in the normal case, and then something different for the SVG element case?

[nfcampos]

yeah sounds about right @lelandrichardson but that something different for SVGs is not testable with our current test setup with jsdom, do we have to test it with karma or something?

[lelandrichardson]

@nfcampos I wouldn't be opposed to having the test suite run with karma as long as we can continue testing the node environment as well (which i believe we could?). This is probably more ideal than testing with jsdom in the first place, though perhaps not as fast.

[ljharb]

Will karma testing work in travis-ci?

[nfcampos]

@lelandrichardson yeah I believe we can keep testing both in node (ran with mocha) and in the browser (ran with karma) with the same test code (here's an example of a repo that runs both mocha and karma https://github.com/jxnblk/reflexbox/blob/master/package.json)
@ljharb seems like it does https://karma-runner.github.io/0.8/plus/Travis-CI.html but i've never tried it there

but I think whatever we do with karma we should continue testing with mocha + jsdom because that's how most people use it I think

[lelandrichardson]

@ljharb it does. you can use chrome headless or phantomjs. I've used both before. Oddly enough, the one thing I haven't done is use karma to run node tests... i wonder if there is a karma-node-runner or something.

[aweary]

I'm 👍 on running the test suite in a browser environment. If our users are doing it, we should be too.

[EvHaus]

Sorry, I'd just like to add one more thing in case it helps track down things. This original issue came out as a result of upgrading from enzyme 2.2.0 to 2.3.0. Downgrading back to 2.2.0 makes my errors go away.

[lelandrichardson]

@EvNaverniouk interesting. that's helpful... we should set up a test case and run git bisect on it.

[aweary]

@lelandrichardson this error was introduced by my first PR that fixed whitespace issues (it introduced the replace call on the class). It was bound to fail eventually as we hadn't expected to get an object, and it basically failed silently before as it would implicitly call toString on the object when it was interpolated.

[lelandrichardson]

oh interesting. so previously it was calling .toString() on the object in the case of the svg element, so it wasn't really "working", but it wasn't throwing an error.

[aweary]

Yeah exactly, so it's not really a regression as much as it is a change that is throwing an error in a situation where we'd really want it to throw since it's not supported just yet.

[chauthai]

I'm getting the same error on Chrome trying to access a dom element with a svg.
How can I help to fix it?

[horyd]

#448

This might be simplifying it a little bit? But it did the trick for me..

[jorilallo]

Ran into this with nvd3 and Phantom.js as well ({baseVal: 'nvd3-svg', animVal: 'nvd3-svg'})

[LINKIWI]

Has there been any progress on this? All of our tests that try to mount an svg are currently disabled for this reason.

[aweary]

@LINKIWI this should be resolved in the latest release thanks to #448, can you try and see?

[LINKIWI]

@aweary #448 does seem to fix this in a headless Electron environment. Thanks for your work!

[aweary]

Thanks for checking! I'm going to close this out then, as it seems it's been resolved 👍

[MoisesCardenas]

I read the solution to use classList instead, but I'm getting confuse can someone please tell me what will be the solution of @EvHaus example using classList?

import {mount} from 'enzyme';
const component = mount(<svg />);
component.find('.test');