[Fix] render: handle Fiber strings and numbers

[ottosichert]

• Fix Incorrect behaviour when statically rendering components returning strings  #2098
• Introduce loadCheerioRoot util
• Use util in render, mount and shallow
• Added test cases in test/staticRender-spec.jsx

[ottosichert]

Rebased and fixed linter errors

[ljharb]

Thanks!

[ljharb]

I've rebased again, including my latest changes as well as your utils spec.

[ottosichert]

That was quick, thank you very much! I didn't see your approval while I was adding the utils spec. I added them because the coverage went down.

With your latest push everything is as it should be 👍

[ottosichert]

This build will fail: https://travis-ci.org/airbnb/enzyme/jobs/585805429

  1) Utils loadCheerioRoot always returns a Cheerio instance:

     AssertionError: expected { Object (options) } to be an instance of 

      at Context.<anonymous> (packages/enzyme-test-suite/test/Utils-spec.jsx:1081:42)

I will push a fix shortly

[ljharb]

cheerio does need to be a dep of the test suite, but that doesn't fix it. I'll wait for your push.

[ottosichert]

I used the public property of the Cheerio constructor called cheerio, see:

https://github.com/cheeriojs/cheerio/blob/1.0.0-rc.3/lib/cheerio.js#L101

Cheerio.prototype.cheerio = '[cheerio object]';

It is only available on Cheerio instances:

$ node
> const cheerio = require('cheerio');
undefined
> cheerio.version
'1.0.0-rc.3'
> cheerio.cheerio
undefined
> cheerio('<div>foo</div>').cheerio
'[cheerio object]'
> cheerio.load('')().cheerio
'[cheerio object]'
> 

[ljharb]

eesh, i mean, that clearly works but that doesn't seem robust enough to be a valuable test. why was instanceOf(cheerio) failing? it works in the repl.

[ottosichert]

The cheerio property has been available since the very first release: https://github.com/cheeriojs/cheerio/blob/0.1.0/src/cheerio.coffee#L66

    cheerio : "0.0.1"   

It was later changed to hold the special value "[cheerio object]" instead of the version number in cheeriojs/cheerio@41337f9

-    cheerio : '0.4.2',
+    cheerio : '[cheerio object]',

As it is also used in dom-serializer https://github.com/cheeriojs/dom-serializer/blob/v0.2.1/index.js#L88 I followed the same approach.

However, I didn't look into why the .instanceOf didn't work as expected, and I can reproduce the same behaviour in node:

> const cheerio = require('cheerio');
undefined
> cheerio.version
'1.0.0-rc.3'
> cheerio.root() instanceof cheerio
true
> cheerio.load('<div>foo</div>', null, false).root() instanceof cheerio
true
> cheerio.load('')('<div>foo</div>') instanceof cheerio
true
> 

I will investigate and submit a new version when I found the issue

[ljharb]

Thanks! The issue isn't whether the property works in every version of cheerio - this is knowable - but whether any object can mistakenly pass the test by having an arbitrary string property. In tests of our own code, this really isn't a big concern, just a philosophical one :-) really, it's that i'd prefer to at least understand why no better option is available before accepting a non-ideal one.

[ottosichert]

What I found out so far is that there are two different identities for cheerio:

• require('cheerio') in enzyme/build/Utils
• import cheerio from 'cheerio' in enzyme-test-suite/test/Utils-spec.jsx

Which means the versions are identical but instanceOf and equality check will fail:

  describe('loadCheerioRoot', () => {
    it('always returns a Cheerio instance', () => {
      const buildCheerio = loadCheerioRoot().constructor;
      expect(buildCheerio.version).to.equal(cheerio.version);
      expect(String(buildCheerio)).to.equal(String(cheerio));
      expect(buildCheerio).to.equal(cheerio); // fails
      [...]

  1) Utils loadCheerioRoot always returns a Cheerio instance:

      AssertionError: expected [Function] to equal [Function]
      + expected - actual

      
      at Context.<anonymous> (packages/enzyme-test-suite/test/Utils-spec.jsx:1084:14)

This seems to be more of a build setup issue, did you run into this problem before?

[ottosichert]

Also reproducible in node:

enzyme/packages/enzyme-test-suite$ node
> const cheerio = require('cheerio');
undefined
> const loadCheerioRoot = require('enzyme/build/Utils').loadCheerioRoot;
undefined
> loadCheerioRoot().constructor.version === cheerio.version
true
> String(loadCheerioRoot().constructor) === String(cheerio)
true
> loadCheerioRoot().constructor === cheerio
false
> 

[ljharb]

gotcha, now that makes sense :-) i'd expect lerna to hoist them up so they're the same version, but either way this is something I can solve. Will fix shortly.

[ljharb]

k, i ended up going with your test suggestion ¯\_(ツ)_/¯

[ottosichert]

@ljharb Thanks for wrapping this up!