[Fix] render: handle Fiber strings and numbers#2221
Conversation
b530e0c to
4ae867f
Compare
|
Rebased and fixed linter errors |
4ae867f to
701ae97
Compare
701ae97 to
674e233
Compare
674e233 to
922a6e3
Compare
922a6e3 to
cf240c8
Compare
|
I've rebased again, including my latest changes as well as your utils spec. |
|
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 👍 |
|
This build will fail: https://travis-ci.org/airbnb/enzyme/jobs/585805429 I will push a fix shortly |
|
cheerio does need to be a dep of the test suite, but that doesn't fix it. I'll wait for your push. |
cf240c8 to
8016139
Compare
|
I used the public property of the 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]'
> |
|
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. |
|
The cheerio : "0.0.1" It was later changed to hold the special value - cheerio : '0.4.2',
+ cheerio : '[cheerio object]',As it is also used in However, I didn't look into why the > 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 |
|
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. |
|
What I found out so far is that there are two different identities for
Which means the versions are identical but This seems to be more of a build setup issue, did you run into this problem before? |
|
Also reproducible in 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
> |
|
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. |
8305b85 to
4b3cc31
Compare
|
k, i ended up going with your test suggestion ¯\_(ツ)_/¯ |
bb08dfb to
9aa653f
Compare
- remove redundant jobs
- `enzyme-adapter-utils` `assertDomAvailable` & `elementToTree` - `EnzymeAdapter` `matchesElementType`
9768184 to
1ac0daf
Compare
cd6e165 to
e27f3bc
Compare
…Wrapper; add tests Technically this could be considered a breaking change since it’s a `class` and these are public methods on it. *However*, since nobody should be depending on this package directly, and since enzyme itself doesn’t appear to use it, I’m going to consider it a patch. In the event that it is a breaking change for someone, I’ll add back the methods, ideally as no-ops.
- [enzyme] [fix] `render`: handle Fiber strings and numbers - [enzyme] [new] `Utils`: add `loadCheerioRoot` Fixes enzymejs#2098
e27f3bc to
d9cc09e
Compare
|
@ljharb Thanks for wrapping this up! |
New Stuff - `render`: handle Fiber strings and numbers (#2221) Fixes - `shallow`: Share child context logic between `shallow` and `dive` (#2296) - `mount`: `children`: include text nodes ($2269) - `mount`: `invoke`: use adapter’s `wrapInvoke` if present (#2158) Docs - `mount`/`shallow`: `closest`/`parent`: Add missing arguments description (#2264) - `mount`/`shallow`: fix pluralization of “exist” (#2262) - `shallow`/`mount`: `simulate`: added functional component example to simulate doc (#2248) - `mount`: `debug`: add missing verbose option flag (#2184) - `mount`/`shallow`: `update`: fix semantics description (#2194) - add missing backticks to linked method names (#2170) - `invoke`: Add missing backticks to end of codeblock (#2160) - `invoke`: Fix typo (#2167) - Explicit React CSS selector syntax description (#2178) Meta Stuff - [meta] add `funding` field - [meta] Update airbnb.io URLs to use https (#2222) - [deps] update `is-boolean-object`, `is-callable`, `is-number-object`, `is-string`, `enzyme-shallow-equal`, `array.prototype.flat`, `function.prototype.name`, `html-element-map`, `is-r egex`, `object-inspect`, `object-is`, `object.entries`, `object.vales`, `raf`, `string.prototype.trim` - [dev deps] update `eslint`, `eslint-plugin-import`, `eslint-plugin-markdown`, `eslint-plugin-react`, `safe-publish-latest`, `eslint-config-airbnb`, `rimraf`, `safe-publish-latest`, `k arma-firefox-launcher`, `babel-preset-airbnb`, `glob-gitignore`, `semver`, `eslint-plugin-jsx-a11y`
loadCheerioRootutilrender,mountandshallowtest/staticRender-spec.jsx