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

Jest Error #85

Closed
tleish opened this issue Dec 12, 2020 · 12 comments
Closed

Jest Error #85

tleish opened this issue Dec 12, 2020 · 12 comments

Comments

@tleish
Copy link

tleish commented Dec 12, 2020

(updated with more backtrace)\

After adding cable_ready package to my project, I can no longer run tests using jest. I get:

 .../node_modules/cable_ready/javascript/cable_ready.js:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,global,jest){import morphdom from 'morphdom'
                                                                                             ^^^^^^

    SyntaxError: Cannot use import statement outside a module

      at ScriptTransformer._transformAndBuildScript (node_modules/@jest/transform/build/ScriptTransformer.js:537:17)
      at ScriptTransformer.transform (node_modules/@jest/transform/build/ScriptTransformer.js:579:25)
      at Object.<anonymous> (node_modules/stimulus_reflex/dist/stimulus_reflex.js:1:123)

Code works when using it, I just can't run javascript tests anymore.

cable_ready 4.4.4

@hopsoft
Copy link
Contributor

hopsoft commented Dec 12, 2020

It looks like this might be related to StimulusReflex rather than CableReady.

@tleish
Copy link
Author

tleish commented Dec 12, 2020

@hopsoft - my mistake, I didn't put the entire backtrace in there (fixed the original post now). StimulusReflex is calling cable_ready.

@tleish
Copy link
Author

tleish commented Dec 13, 2020

A standard practice for node package is to include both ESM and CommonJS in the same package. Many put the ESM in a source directory and the CommonJS in a dist directory, with the details in package.json. Doing this would make it easy to run a test suite like Jest while still including the ESM package for more fine grain builds.

@andrewmcodes
Copy link
Contributor

Agreed - didn't realize we weren't. Are you interested in taking a shot at it?

@marcoroth
Copy link
Member

marcoroth commented Dec 13, 2020

We did that in the past with microbundle in StimulusReflex but reverted the change because it didn't work when you tried to link the package locally or wanted to use sourcemaps. I also reverted this change in #77 (ee3cdc6) for CableReady.

Now if I'm thinking about it: we don't need the CommonJS version during development would just need to build the CommonJS version as soon as we release a new version so it would be included in the released version.

@tleish
Copy link
Author

tleish commented Dec 13, 2020

It looks like StimulusReflex does this in it's package.

package.json

{
  "source": "./stimulus_reflex.js",
  "main": "./dist/stimulus_reflex.js",
  "module": "./dist/stimulus_reflex.module.js",
  "esmodule": "./dist/stimulus_reflex.modern.js",
}

source tree

.
├── dist
│   └── stimulus_reflex.js
├── package.json
└── stimulus_reflex.js

@marcoroth
Copy link
Member

marcoroth commented Dec 13, 2020

Which version is that @tleish? This is how it looked in a previous pre release of 3.4 if I'm not mistaken.
The current master looks different: https://github.com/hopsoft/stimulus_reflex/blob/master/package.json#L31

Or is npm doing some magic processing when the package is released?

@tleish
Copy link
Author

tleish commented Dec 13, 2020

Looks like it's 3.4.0-pre4.

@tleish
Copy link
Author

tleish commented Dec 13, 2020

After upgrading StimulusReflex to the latest, I now get the same error... just on stimulus_reflex (before it imports cable_ready):

So, both packages now have similar challenges when testing a project which imports them as modules.

node_modules/stimulus_reflex/javascript/stimulus_reflex.js:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,global,jest){import { Controller } from 'stimulus'
                                                                                             ^^^^^^

    SyntaxError: Cannot use import statement outside a module

    > 1 | import { Controller } from 'stimulus'
        | ^

stimulus_reflex worked before when there was a CommonJS version.

@tleish
Copy link
Author

tleish commented Dec 13, 2020

Ok, I have a workaround that I believe it working now. It took a bit to get to the resolution. I still think both stimulus_reflex and cable_ready should distribute a CommonJS, but I'll give a bit of history how I got there.

Running test through jest with stimulus_reflex 3.4.0-pre4 and cable_ready (latest)

Pointed the error to cable_ready.

I updated the jest framework configuration (jest.config.js) with:

{
-   transformIgnorePatterns: ['<rootDir>/node_modules/(?!(@babel))'],
+   transformIgnorePatterns: ['<rootDir>/node_modules/(?!(@babel|cable_ready))'],
}

see: Configuring Jest > transformIgnorePatterns

Jest ignores node_modules directory for transformations as it assumes all node_modules have a dist package. This setting tells jest which node_module files NOT to ignore. Making this config change caused thousands of lines of error code in the console when testing. My guess now is stimulus_reflex 3.4.0-pre4 (CommonJS) importing module cable_ready (ESM) caused this error.

Upgrading to stimulus_reflex (latest)

Pointed the error to stimulus_reflex.

I updated the jest framework configuration to:

{
-   transformIgnorePatterns: ['<rootDir>/node_modules/(?!(@babel))'],
+   transformIgnorePatterns: ['<rootDir>/node_modules/(?!(@babel|stimulus_reflex))'],
}

This now pointed the error to cable_ready, so I updated the jest framework configuration to:

{
-   transformIgnorePatterns: ['<rootDir>/node_modules/(?!(@babel|stimulus_reflex))'],
+   transformIgnorePatterns: ['<rootDir>/node_modules/(?!(@babel|stimulus_reflex|cable_ready))'],
}

No more errors 🎉 !!!

I'll point out I've been using jest testing with lots of node modules for a while, and this is the first time I've had to tell jest to include node_module (other than babel) when transpiling. I have multiple node dependencies in this project, and do not need to add this for the other libraries.

@marcoroth
Copy link
Member

Thanks @tleish for digging into this! Awesome that you found a workaround that works for now.
It's definitely not setup correctly so let's fix this for everyone 😊 I guess the problem is, that the main field in the package.json points to an es module.

@leastbad
Copy link
Contributor

Hey @tleish I'm going to close this because the docs are a better place for this kind of project wisdom.

Feel free to jump on Discord and hit #docs if you'd like to help me translate what you're doing to something generally applicable. Thanks for sharing!

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

No branches or pull requests

5 participants