Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v3.2.15 regression with babel/register and jest #747

Closed
dnalborczyk opened this issue Mar 12, 2019 · 9 comments
Closed

v3.2.15 regression with babel/register and jest #747

dnalborczyk opened this issue Mar 12, 2019 · 9 comments

Comments

@dnalborczyk
Copy link
Contributor

dnalborczyk commented Mar 12, 2019

the following works with esm v3.14.0 and less, fails with v3.15.0+:

// some.test.js
require = require('esm')(module)
require('@babel/register')
const { someFunc } = require('../')

test('some test', () => {
  expect(true).toBe(true)
})
Test suite failed to run

    /dev/node_modules/@babel/traverse/lib/visitors.js:1
    Error: [BABEL] /dev/packages/component/src/utils/index.js: You gave us a visitor for the node type constructor but it's not a valid type (While processing: "/dev/node_modules/@babel/plugin-proposal-class-properties/lib/index.js")
stack trace

      at verify (../../node_modules/@babel/traverse/lib/visitors.js:130:13)
      at explode (../../node_modules/@babel/traverse/lib/visitors.js:52:3)
      at Object.merge (../../node_modules/@babel/traverse/lib/visitors.js:165:5)
      at Object.<anonymous> (../../node_modules/@babel/helper-replace-supers/lib/index.js:89:46)
      at Object._compile (../../node_modules/pirates/lib/index.js:99:24)
      at Object.newLoader [as .js] (../../node_modules/pirates/lib/index.js:104:7)
      at _helperReplaceSupers (../../node_modules/@babel/helper-create-class-features-plugin/lib/fields.js:22:40)
      at Object.<anonymous> (../../node_modules/@babel/helper-create-class-features-plugin/lib/fields.js:156:4)
      at Object._compile (../../node_modules/pirates/lib/index.js:99:24)
      at Object.newLoader [as .js] (../../node_modules/pirates/lib/index.js:104:7)
      at Object.<anonymous> (../../node_modules/@babel/helper-create-class-features-plugin/lib/index.js:34:15)
      at Object._compile (../../node_modules/pirates/lib/index.js:99:24)
      at Object.newLoader [as .js] (../../node_modules/pirates/lib/index.js:104:7)
      at _helperCreateClassFeaturesPlugin (../../node_modules/@babel/plugin-proposal-class-properties/lib/index.js:19:16)
      at _default (../../node_modules/@babel/plugin-proposal-class-properties/lib/index.js:30:14)
      at ../../node_modules/@babel/helper-plugin-utils/lib/index.js:19:12
      at loadDescriptor (../../node_modules/@babel/core/lib/config/full.js:165:14)
      at cachedFunction (../../node_modules/@babel/core/lib/config/caching.js:33:19)
      at loadPluginDescriptor (../../node_modules/@babel/core/lib/config/full.js:200:28)
      at config.plugins.reduce (../../node_modules/@babel/core/lib/config/full.js:69:20)
          at Array.reduce (<anonymous>)
      at recurseDescriptors (../../node_modules/@babel/core/lib/config/full.js:67:38)
      at loadFullConfig (../../node_modules/@babel/core/lib/config/full.js:108:6)
      at loadOptions (../../node_modules/@babel/core/lib/config/index.js:27:36)
      at OptionManager.init (../../node_modules/@babel/core/lib/index.js:231:36)
      at compile (../../node_modules/@babel/register/lib/node.js:61:42)
      at compileHook (../../node_modules/@babel/register/lib/node.js:102:12)
      at Object._compile (../../node_modules/pirates/lib/index.js:93:29)


@jdalton I can look into creating a test case if you want to keep supporting this scenario.

(changed versions)

@jdalton

This comment has been minimized.

@jdalton jdalton closed this as completed Mar 12, 2019
@jdalton jdalton reopened this Mar 12, 2019
@dnalborczyk

This comment has been minimized.

@dnalborczyk dnalborczyk changed the title v3.2.16 regression with babel/register and jest v3.2.15 regression with babel/register and jest Mar 12, 2019
@dnalborczyk

This comment has been minimized.

@jdalton
Copy link
Member

jdalton commented Mar 12, 2019

I'm guessing this change.

Could you create a repro repo?

I can look into creating a test case if you want to keep supporting this scenario.

Yes, please! Thank you!

@dnalborczyk
Copy link
Contributor Author

@jdalton on second thought, I think I was using jest not the way it was intended.

jest is transforming files implicitly with babel, unless overwritten:
https://jestjs.io/docs/en/configuration.html#transform-object-string-string

it seems bridging with esm somehow invalidates the transform mechanism, hence I explicitly used @babel/register in the test script.

my gut feeling tells me it should be left alone, until esm supports jest officially through transforms. what do you think?

til then I just let jest using babel for es6 module transforms:

// babel.config.js
const {
  env: { NODE_ENV },
} = process

const isTest = NODE_ENV === 'test'
const plugins = ['@babel/plugin-transform-flow-strip-types']

if (isTest) {
  plugins.push('@babel/plugin-transform-modules-commonjs')
}

module.exports = {
  plugins,
}

@jdalton
Copy link
Member

jdalton commented Mar 12, 2019

If the scenario worked before and now doesn't I'd still like a repro to investigate.

Update:

Related issue nuxt/nuxt#5203 (comment)

Update:

From 3.2.14...3.2.15 I think maybe this could be the problem patch dd7d96f. It moved setting the prototype after setting properties. I did this because I thought it would allow setting more properties (since if a non-writable property exists on a prototype it will not be written as an own property either). However, it looks like these added properties are causing problems.

Update:

Or more likely this change fc232ac#diff-5708a2c960a8a812847884b65cd4590fR154. As it changes the setting of the constructor as a plain assignment which would preserve non-enumerability to one that forces enumerability.

@dnalborczyk
Copy link
Contributor Author

dnalborczyk commented Mar 13, 2019

hey @jdalton , sorry, been too busy yesterday. looking into repro now ....

update:
here you go: https://github.com/dnalborczyk/jest-esm-regression

npm test

I omitted any babel-plugin, babel.config.js etc. as it doesn't seem to be needed to cause the exception.

@ForsakenHarmony
Copy link

Also ran into a related issue with webpack validation

configuration.optimization.splitChunks has an unknown property 'constructor'.

@jdalton
Copy link
Member

jdalton commented Mar 13, 2019

@dnalborczyk Thanks for the repro. Confirmed the patch has fixed it.

Update

esm v3.2.17 is released 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants