-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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 to html output changed in v3 #1162
Comments
We upgraded from cheerio v0.22 to v1, so breaking changes are expected - https://github.com/airbnb/enzyme/blob/master/docs/guides/migration-from-2-to-3.md#cheerio-has-been-updated-thus-render-has-been-updated-as-well - in this case, it's due to https://github.com/cheeriojs/cheerio/blob/48eae25c93702a29b8cd0d09c4a2dce2f912d1f4/History.md#migrating-from-version-0x. In other words, the render wrapper is now around what the component renders, so You can, however, (cc @jugglinmike) |
@ljharb i'd like to understand this better, but this feels wrong to me still... I think the above test probably shouldn't have broken in this release :/ |
It would have been ideal, but the breaking changes in cheerio are such that it was unavoidable - they no longer have the concept of "root", and |
I can confirm, Given that, would it be possible to return a wrapper around cheerio for the
|
@mpeyper 1) that would make the |
Obviously you know the cheerio api better than I do so I'll leave the actual implementation up to you guys. All I was suggesting was that if the cheerio api isn't ideal anymore, perhaps you could provide your own that uses cheerio under the hood. |
This breaking change is really a bummer, and a step backwards IMHO. I understand your concerns with cheerio, but to my mind this is an implementation detail that enzyme should solve to keep BC. I use enzyme, not cheerio. If
it('should render a large paragraph', () => {
const Test = () => <p style={{ fontSize: 13 }}>test</p>
expect(render(<Test />).html()).to.equal('<p style="font-size: 13px">test</p>');
}) Also, writing tests like the following really doesn't make sense: it('should render html', () => {
const Test = () => <p>test</p>
expect(render(<Test />).html()).to.equal("test")
}); |
it('should render a large paragraph', () => {
const Test = () => <p style={{ fontSize: 13 }}>test</p>;
const wrapper = render(<Test />);
expect(wrapper.is('p')).to.equal(true);
expect(wrapper.attr('style')).to.equal('font-size: 13px;');
expect(render(<Test />).html()).to.equal('test');
}); Indeed, the docs should be updated. In order to make a change here, something would have to change inside cheerio itself. |
After those new changes the whole expect(render(<Test/>).html()).to.equal('<p class="p-class">test</p>'); Now I have to use 'render' API for that which is an overkill in 80% of cases. It would be nice if you could bring the outer HTML back at some form. |
I use render().html() for snapshot, And it would be nice if you could bring the outer HTML back at some form~~~ |
In cheerio, it looks like had this discussion back in 2012 and came up with there'a slso |
This has the unintended consequence that it makes const base = <div><b>important</b></div>;
it('sanity test', () => {
expect( mount(base).html()).toEqual('<div><b>important</b></div>');
expect( render(base).html()).toEqual('<b>important</b>');
expect(shallow(base).html()).toEqual('<div><b>important</b></div>');
}); Which is... very confusing for someone getting started with Enzyme, and adds a lot of extra sanity checks along the way 😢 IMHO, it should be either the equivalent of |
@franciscop-invast If you are using |
I am actually not rendering the Thanks for the tip @mykhailo-riabokon , but the snapshots seems very strict while I'm only interested in testing few things, so Enzyme works better for me (thanks for making it!). |
@franciscop-invast I think .debug() method should have worked better for this case, as And as for snapshot testing, I have used it for a while and it's not strict at all :) And if you want to scale your assertions in one day (I mean test a bit more than a few things) you already have a solution in place. |
enzyme doesn't support snapshots, but |
I'm going to close this - however if anyone in the future can provide a PR that would address the inconsistencies with render vs shallow/mount, I'd be happy to review it. |
Previously using v2.9.1 and moving to v3.0.0, the output from
render(...).html()
has changed.Previously, the following test would pass:
In v3, I get the following error:
I have checked the migration guide and change log for both enzyme and cheerio, but could not find reference to this being intentional.
The text was updated successfully, but these errors were encountered: