Skip to content
This repository has been archived by the owner on Oct 15, 2020. It is now read-only.

jest-haste-map 22.4.0 breaks in node-chakracore #477

Closed
jdalton opened this issue Feb 24, 2018 · 13 comments
Closed

jest-haste-map 22.4.0 breaks in node-chakracore #477

jdalton opened this issue Feb 24, 2018 · 13 comments

Comments

@jdalton
Copy link
Member

jdalton commented Feb 24, 2018

Just a heads up. I have a unit test that checks jest + mock-require and it started failing after with the `jest-haste-map 22.4.0 update (on Windows 10).

\cc @SimenB

@jdalton
Copy link
Member Author

jdalton commented Feb 24, 2018

It is probably related to this bit jestjs/jest@3a2854a#diff-9450b62547fd20fe4b1669dd088f14af.

The error I get is

TypeError: Unable to get property 'files' of undefined or null reference
   at Anonymous function (C:\projects\esm\node_modules\jest-haste-map\build\index.js:324:7)

It's reading a haste cache file but the file is empty, "".

@SimenB
Copy link
Member

SimenB commented Feb 24, 2018

Can you try with 22.4.2? Ref jestjs/jest#5642.

Doesn't chakra node support v8.serialize? Which would be understandable, but then we shouldn't hit that code path anyways.

Underlying change: jestjs/jest#5565

@jdalton
Copy link
Member Author

jdalton commented Feb 24, 2018

I actually worked backwards from 22.4.2. The first version to work again was 22.3.0.

@SimenB
Copy link
Member

SimenB commented Feb 24, 2018

/cc @mjesun

@MSLaguana MSLaguana added the bug label Feb 26, 2018
@MSLaguana
Copy link
Contributor

The v8.deserialize method seems to have nothing to do with v8, and it's implemented in node so we should support it. It looks to me like right now the native methods it calls into are being mis-compiled into do-nothing functions, so it is returning undefined.

@MSLaguana
Copy link
Contributor

Ah nevermind; the node_serdes serialization methods end up depending on a v8 type that we don't mock out, and so the compiler was calling our bluff.

@mjesun
Copy link

mjesun commented Feb 26, 2018

Sorry for the late reply, I just got an email about that (thanks, GitHub...). From what you say it looks like it's not a particular bug in Jest?

What would work as well for you is if you don't provide v8.serialize at all. In that case, we default back to an implementation with a standard JSON.stringify.

@MSLaguana
Copy link
Contributor

The difficulty with that is we kind-of/kind-of-don't support everything in the v8.js file. The simplest thing from our side would be to remove v8.js, but then your require("v8") would throw and that looks like it'd be worse for you at the moment. The alternative that you propose is essentially having v8.js as an empty file, which isn't too unreasonable, but as a general rule we try to keep our diff from upstream node as small as possible.

@digitalinfinity
Copy link
Contributor

I think it's not unreasonable for v8.js to just return an empty object if process.jsEngine != 'v8'. @addaleax thoughts? (and yes, we probably should get around to supporting serdes at some point, it's still on my backlog 😞)

@mjesun
Copy link

mjesun commented Feb 27, 2018

@MSLaguana I can wrap the V8 require in a try/catch prior to check v8.serialize; would that make things simpler?

@MSLaguana
Copy link
Contributor

I've just put out a PR to have the v8 module be an empty object: #491

@MSLaguana
Copy link
Contributor

This should be fixed in nightlies now, triggering the fallback to JSON.parse

@kfarnung
Copy link
Contributor

The fix was also backported to our latest release build: https://github.com/nodejs/node-chakracore/releases/tag/node-chakracore-v8.10.0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants