reference mismatch with esm imports and hapijs/lab

[dnalborczyk]

Just for fun I pulled down and converted hapi to use es modules with esm, and to my surprise, the test suite running with lab converted to use es-modules worked out-of-the-box with esm as well !! Really amazing!!! 😃

the test suite produced one single bug. Not sure if that is a lab-specific issue.

repro in the works ...

speaking of amazing: debugging the failed test case with ndb was short of amazing as well!! just worked out-of-the-box with esm!!

[jdalton]

Thanks for digging in! Do you know off hand what the reference mismatch was with.
I've seen it in the past with some things so have some idea.

[dnalborczyk]

I literally thought I had it ... and then it slipped away.
sometimes I have the feeling esm is self-healing 😃

I might have to check in the converted hapi repo for you to have a look at.

edit:

I think I found the culprit. I think I was "double loading" esm through api and cli. some tests pointed to the bridge, while others didn't (cli). might still be a bug. still looking ...

[dnalborczyk]

here is a repo of the above "issue", although I'm not sure if it really is an issue - but it might help to uncover some underlying or hidden problem when esm is being used programmatically and with the cli.

https://github.com/dnalborczyk/esm-repros/tree/issue-524

when you run the tests with npm test, they will pass. you have to comment out this line to make it fail.

[jdalton]

Cool. What version of Node are you testing on?

[dnalborczyk]

sorry, latest, v10.7.0

btw, forgot to mention: it really is 1 test, the other "empty" test can easily be removed. just didn't wanna confuse you.

edit: just noticed that the repo is using a mix of Stream function and a Stream property on the Stream function, but I believe both are the same: require('stream') === require('stream').Stream

[dnalborczyk]

btw, the hapi tests with lab are fully succeeding!!

@jdalton don't spend any time on this if you see no value. it's clearly being used in a way it wasn't meant to be.

[jdalton]

This is an interesting issue in how we share and access state. It's a legit bug. In the deferred getter for the builtin entries I don't check if the entry has already been created (I assume if it had a deferred getter added that means it wasn't found in the cache). However, in this case the deferred getter is added in one place and before the getter is tripped (to create the entry) the entry is created in another.
Checking the builtin cache before creation on the getter resolves the issue.

[jdalton]

test debt 💰💰

Update:

@dnalborczyk I'm adding a new label to issues that I have not added tests for. You can follow along with this query. We'll remove the label from issues as the test addition PRs are merged.

[dnalborczyk]

@jdalton good idea! 👍

[jdalton]

@dnalborczyk I fixed the query link. Some of them, like the Electron ones, aren't practical to test in an automated way but are good to be aware of.

Update:

v3.0.73 is released 🎉