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

Render Testing dangerouslySetInnerHTML containing an xlink #1298

Closed
peter-mouland opened this issue Oct 24, 2017 · 9 comments
Closed

Render Testing dangerouslySetInnerHTML containing an xlink #1298

peter-mouland opened this issue Oct 24, 2017 · 9 comments

Comments

@peter-mouland
Copy link
Contributor

peter-mouland commented Oct 24, 2017

I have recently rewrote a test (from v2 to v3) and found now that xlink is removed from the dangerouslySetInnerHTML ...

I'm not convinced this is really enzyme removing this, but it does seem to render correctly in the browser.

Have i found an issue, or am I missing something?

Thanks for your time.

I found this while recoding for the issue #1297 in order to separate the 2

Example component:

// svg.js
import React from 'react'

export default ({ height, width, children, className, symbol = false, id, use, ...props }) => {
  const isBase64 = typeof children === 'string' && children.indexOf('data') === 0
  let svg = use ? `<svg ><use xlink:href='#${use}' /></svg>` : children
  if (id) svg = svg.replace(/(<svg[^>]*) id=".*?"/ig, '$1').replace('<svg', `<svg id="${id}"`)
  if (height) svg = svg.replace(/(<svg[^>]*) height=".*?"/ig, '$1').replace('<svg', `<svg height="${height}"`)
  if (width) svg = svg.replace(/(<svg[^>]*) width=".*?"/ig, '$1').replace('<svg', `<svg width="${width}"`)
  if (symbol) svg = svg.replace('<svg', '<svg style="display: none;" ><symbol')
  return isBase64
    ? <img src={svg} className={className} {...props} />
    : <span dangerouslySetInnerHTML={{ __html: svg }} className={className} {...props} />
}

Failing Test

// svg.spec.js


import React from 'react'
import { shallow, render } from 'enzyme'
import Chance from 'chance'

import Svg from './Svg'

const chance = new Chance()
const svg = '<svg class="test-svg" xmlns="http://www.w3.org/2000/svg" />'
const defaultMockProps = {
  id: chance.word()
}

describe('Svg', () => {
  it('renders the use href using xlink (needed for safari)', () => {
    const component = render(<Svg use={defaultMockProps.id} />)
    expect(component.html()).toContain(`xlink:href="#${defaultMockProps.id}"`)
  })

Result:

    Expected string:
      "<svg><use href=\"#ku\"/></svg>"
    To contain value:
      "xlink:href=\"#ku\""
      

React:

<svg><use xlink:href="#my-test"></use></svg>

This test works using shallow:

  it('renders the use href using xlink (needed for safari)', () => {
    const component = shallow(<Svg use={defaultMockProps.id} />)
    expect(component.html()).toContain(`xlink:href='#${defaultMockProps.id}'`)
  })
@peter-mouland peter-mouland changed the title Testing dangerouslySetInnerHTML containing an xlink Render Testing dangerouslySetInnerHTML containing an xlink Oct 24, 2017
@ljharb
Copy link
Member

ljharb commented Oct 25, 2017

It seems like the kind of nonstandard property that React itself is removing. What version of react?

@peter-mouland
Copy link
Contributor Author

This was with my upgrade from React 15 (tested w/ enzyme v2) to React 16 (tested w/ enzyme v3)

@ljharb
Copy link
Member

ljharb commented Oct 25, 2017

@peter-mouland by upgrading both react and enzyme at once, it's pretty hard to know where the problem is. Could you get enzyme 3 with react 15 passing first, and then upgrade to React 16?

@peter-mouland
Copy link
Contributor Author

peter-mouland commented Oct 25, 2017

sure, that's fair - i'm in the UK though so not right now ;)
hopefully get a chance to do it tomorrow or Friday (depending on work!), but shouldn't take long just to change the adapter. will let you know

@peter-mouland
Copy link
Contributor Author

peter-mouland commented Oct 26, 2017

I can confirm:

tests

  it('renders the use href using xlink (needed for safari) original', () => {
    const component = render(<Svg use={defaultMockProps.id} />)
    const use = component.find('use')
    expect(use.prop('xlink:href')).toBe(`#${defaultMockProps.id}`)
  })

  it('renders the use href using xlink (needed for safari) w/ render', () => {
    const component = render(<Svg use={defaultMockProps.id} />)
    expect(component.html()).toContain(`xlink:href="#${defaultMockProps.id}"`)
  })

  it('renders the use href using xlink (needed for safari) w/ shallow', () => {
    const component = shallow(<Svg use={defaultMockProps.id} />)
    expect(component.html()).toContain(`xlink:href="#${defaultMockProps.id}"`)
  })

result: enzyme 2 and React 15.6.2

// original
*pass*

// w/render   
*pass*

// w/shallow
*pass*

result: enzyme 3 React 15 (same result with React 16)

// original
    Expected value to be (using ===):
      "#wugjaco"
    Received:
      undefined

// w/render   
    Expected string:
      "<svg><use href=\"#wugjaco\"/></svg>"
    To contain value:
      "xlink:href=\"#wugjaco\""

// w/shallow
*pass*

(original fails as find('use').prop('xlink:href') returns length 0)

@ljharb
Copy link
Member

ljharb commented Oct 26, 2017

Thanks! That means it's not specific to the react 16 adapter, which means it's a v3 issue.

@aweary, could you take a look into this one?

@chenesan
Copy link
Contributor

Hi @ljharb I'd like to look into this. I may try to fix issues about selectors recently.

@chenesan
Copy link
Contributor

Provide a simpler test case:

it('render `xlink:href` in html of dangerouslySetInnerHTML', () => {
      const xlinkId = 'TEST';
      const svg = `<svg><use xlink:href="#${xlinkId}" /></svg>`;
      const wrapper = render(<span dangerouslySetInnerHTML={{ __html: svg }} />);

      expect(wrapper.html()).contain(`xlink:href="#${xlinkId}"`);
    });

It looks like it's an issue in cheerio side. cheerio converts xlink:href attribute to href after 1.0.0.
In enzyme v2 we used cheerio@^0.22.0 so the above test will pass.

See cheeriojs/cheerio#1101 .

@ljharb
Copy link
Member

ljharb commented Dec 18, 2018

In that case, closing in favor of #1297 which already is tracking that cheerio issue.

@ljharb ljharb closed this as completed Dec 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants