Esm: Possible regression from 3.0.84

Created on 17 Jan 2019  路  8Comments  路  Source: standard-things/esm

Here鈥檚 a PR bumping esm from 3.0.84 to 3.1.0 and it failing. This is the failing run.

The error is (0 , _prompt.default) is not a function.

When I look at this import statement in the test, the value returned has changed on 3.1.0.

On 3.0.84 it logs as [Function: default]
On 3.1.0 it logs as { default: [Function: default] }

I鈥檓 running esm through AVA鈥檚 require hook.

Is there any other information that would be helpful?

bug

Most helpful comment

After more digging the sub-loaded detection was a false lead. The issue is the assumptions we make about CJS files being loaded. In this case

"core-js/fn/array/flat-map",
"core-js/fn/array/flatten"

We just need to track that they are being loaded via the esm enabled require.

Update:

I have this fixed locally. Ended up not needing to track extra things.

Update:

Patched https://github.com/standard-things/esm/commit/992adb99dec9d2ee76119cee24626ef0de031296; Tests https://github.com/standard-things/esm/commit/1c1aca09eb6e510597a7627ed42ba081541ad482.

All 8 comments

Thanks @brandonweiss!

Looks like a possible regression in our __esModule+Babel interop. Should be a straight forward fix. You can expect a patch release this evening to resolve it.

for what it's worth:

I changed the PR to use Ava v1.0.0-beta.7 (same version esm is using in it's scenario tests, and added the babel-options to turn off the default esmodules-to-cjs-conversion. the docs are a bit unclear about this, if it should be, or even has to be done: https://github.com/avajs/ava/blob/master/docs/recipes/es-modules.md https://github.com/avajs/ava/blob/master/docs/recipes/babel.md#preserve-es-module-syntax

tests are passing then.

  "ava": {
    "require": [
      "esm",
      "core-js/fn/array/flat-map",
      "core-js/fn/array/flatten"
    ],
    "babel": {
      "testOptions": {
        "presets": [
          [
            "module:ava/stage-4",
            {
              "modules": false
            }
          ]
        ]
      }
    }
  }

I've upgraded our scenario test dep to "ava": "^1.1.0" and tests still pass. I'll look into adding another scenario test.

@jdalton Thanks! 鉂わ笍

Okay! So it wasn't what I thought and is a little trickier to fix. It has to do with our detection for sub-loaded deps, deps loaded during the compilation of others _(this is done by things like babel/register or nyc)_.

@jdalton does esm need to detect if other deps have been loaded? is it not sufficient to apply e.g. options such as "modules": false? to e.g. @babel/register, ava etc.?

After more digging the sub-loaded detection was a false lead. The issue is the assumptions we make about CJS files being loaded. In this case

"core-js/fn/array/flat-map",
"core-js/fn/array/flatten"

We just need to track that they are being loaded via the esm enabled require.

Update:

I have this fixed locally. Ended up not needing to track extra things.

Update:

Patched https://github.com/standard-things/esm/commit/992adb99dec9d2ee76119cee24626ef0de031296; Tests https://github.com/standard-things/esm/commit/1c1aca09eb6e510597a7627ed42ba081541ad482.

@jdalton Thanks! 鉂わ笍

Was this page helpful?
0 / 5 - 0 ratings

Related issues

OmgImAlexis picture OmgImAlexis  路  3Comments

kherock picture kherock  路  3Comments

Mensu picture Mensu  路  3Comments

janusqa picture janusqa  路  3Comments

mAAdhaTTah picture mAAdhaTTah  路  3Comments