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

WP Scripts: Syntax error when running Jest #43132

Closed
ryelle opened this issue Aug 10, 2022 · 14 comments
Closed

WP Scripts: Syntax error when running Jest #43132

ryelle opened this issue Aug 10, 2022 · 14 comments
Labels
Needs Dev Ready for, and needs developer efforts npm Packages Related to npm packages [Package] Jest preset /packages/jest-preset-default [Package] Scripts /packages/scripts [Type] Bug An existing feature does not function as intended
Projects

Comments

@ryelle
Copy link
Contributor

ryelle commented Aug 10, 2022

Description

After updating packages, I'm getting a syntax error in my project when running wp-scripts test-unit-js.

Step-by-step reproduction instructions

Run wp-scripts test-unit-js on a project that, somewhere along the way, imports the is-plain-obj package.

● Test suite failed to run

    Jest encountered an unexpected token

    Jest failed to parse a file. This happens e.g. when your code or its dependencies use non-standard JavaScript syntax, or when Jest is not configured to support such syntax.

    Out of the box Jest supports Babel, which will be used to transform your files into valid JS based on your Babel configuration.

    By default "node_modules" folder is ignored by transformers.

    Here's what you can do:
     • If you are trying to use ECMAScript Modules, see https://jestjs.io/docs/ecmascript-modules for how to enable it.
     • If you are trying to use TypeScript, see https://jestjs.io/docs/getting-started#using-typescript
     • To have some of your "node_modules" files transformed, you can specify a custom "transformIgnorePatterns" in your config.
     • If you need a custom transformation specify a "transform" option in your config.
     • If you simply want to mock your non-JS modules (e.g. binary assets) you can stub them out with the "moduleNameMapper" config option.

    You'll find more details and examples of these config options in the docs:
    https://jestjs.io/docs/configuration
    For information about custom transformations, see:
    https://jestjs.io/docs/code-transformation

    Details:

    /Users/ryelle/Projects/Work/pattern-directory/node_modules/is-plain-obj/index.js:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,jest){export default function isPlainObject(value) {
                                                                                      ^^^^^^

    SyntaxError: Unexpected token 'export'

      at Runtime.createScriptFromCode (node_modules/jest-runtime/build/index.js:1728:14)
      at Object.<anonymous> (node_modules/@wordpress/redux-routine/build/@wordpress/redux-routine/src/is-action.js:4:1)

I've fixed it for now by creating a local jest.config.js file with the following. I copied transformIgnorePatterns out of the gutenberg jest.config.js. I think adding that line to the jest-unit.config.js file in @wordpress/scripts would fix the issue without requiring custom code.

const defaultConfig = require( '@wordpress/scripts/config/jest-unit.config' );

const config = {
	...defaultConfig,
	transformIgnorePatterns: [ 'node_modules/(?!(is-plain-obj))' ],
};

module.exports = config;

Screenshots, screen recording, code snippet

No response

Environment info

@wordpress/scripts 23.7.0

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

N/A

@ryelle ryelle added the [Package] Scripts /packages/scripts label Aug 10, 2022
ryelle added a commit to WordPress/pattern-directory that referenced this issue Aug 10, 2022
@ryelle
Copy link
Contributor Author

ryelle commented Aug 10, 2022

Well, my fix worked locally but isn't working in github actions, so you can see the broken tests here.

@gziolo gziolo added [Type] Bug An existing feature does not function as intended npm Packages Related to npm packages labels Aug 11, 2022
@gziolo
Copy link
Member

gziolo commented Aug 12, 2022

@tyxla, it looks like using ESM code in 3rd party packages is more complicated with Jest than we anticipated. I’m wondering what next steps we should take. As a short term solution we can enable transformation for this package in Jest preset. Technically speaking, we could use a different package, but it’s rather a more general issue that will come back as ESM becomes more popular.

The most challenging part is that ESM isn’t fully supported by Jest. See jestjs/jest#9430 and https://jestjs.io/docs/ecmascript-modules. They use some special vm API for ESM that is still experimental in Node according to nodejs/node#37648.

@gziolo gziolo added the [Package] Jest preset /packages/jest-preset-default label Aug 12, 2022
@gziolo gziolo added this to Triage in Core JS via automation Aug 12, 2022
@gziolo gziolo added the Needs Dev Ready for, and needs developer efforts label Aug 12, 2022
@gziolo gziolo moved this from Triage to To do in Core JS Aug 12, 2022
@tyxla
Copy link
Member

tyxla commented Aug 12, 2022

Thanks for reporting this, @ryelle!

Yeah, this was expected to happen, and it's a bummer that we have limited options to fix it.

As you mentioned, a quick workaround for now is to move the transform ignore rule to the default jest preset, and that way it will be considered by the scripts package. Let me file a PR.

@tyxla
Copy link
Member

tyxla commented Aug 12, 2022

Here's a PR that should solve the problem: #43179.

Once we merge it, we'll only need to publish new minor versions of the affected packages.

@gziolo
Copy link
Member

gziolo commented Aug 12, 2022

#43179 should be available on npm in a few minutes. It should resolve the issue.

@gziolo gziolo closed this as completed Aug 12, 2022
Core JS automation moved this from To do to Done Aug 12, 2022
ryelle added a commit to WordPress/pattern-directory that referenced this issue Aug 15, 2022
The Jest config was a workaround for this issue: WordPress/gutenberg#43132 — now that this is fixed in @wordpress/scripts 23.7.1, it's not necessary.
@herndlm
Copy link

herndlm commented Aug 16, 2022

Weird, I have a project were I still get this error, even after updating jest-preset-default to the latest version. I noticed the path is different though than the one initially reported. Also, we're using TS and have the following custom jest config

// eslint-disable-next-line import/no-extraneous-dependencies
const defaults = require('@wordpress/scripts/config/jest-unit.config');

module.exports = {
  ...defaults,
  clearMocks: true,
  coveragePathIgnorePatterns: [
    '<rootDir>/lib/',
    '<rootDir>/node_modules/',
  ],
};

The error is

Jest encountered an unexpected token

    Jest failed to parse a file. This happens e.g. when your code or its dependencies use non-standard JavaScript syntax, or when Jest is not configured to support such syntax.

    Out of the box Jest supports Babel, which will be used to transform your files into valid JS based on your Babel configuration.

    By default "node_modules" folder is ignored by transformers.

    Here's what you can do:
     • If you are trying to use ECMAScript Modules, see https://jestjs.io/docs/ecmascript-modules for how to enable it.
     • If you are trying to use TypeScript, see https://jestjs.io/docs/getting-started#using-typescript
     • To have some of your "node_modules" files transformed, you can specify a custom "transformIgnorePatterns" in your config.
     • If you need a custom transformation specify a "transform" option in your config.
     • If you simply want to mock your non-JS modules (e.g. binary assets) you can stub them out with the "moduleNameMapper" config option.

    You'll find more details and examples of these config options in the docs:
    https://jestjs.io/docs/configuration
    For information about custom transformations, see:
    https://jestjs.io/docs/code-transformation

    Details:

    <project-dir>/node_modules/@wordpress/element/node_modules/is-plain-obj/index.js:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,jest){export default function isPlainObject(value) {
                                                                                      ^^^^^^

    SyntaxError: Unexpected token 'export'

      at Runtime.createScriptFromCode (node_modules/jest-runtime/build/index.js:1728:14)
      at Object.<anonymous> (node_modules/@wordpress/element/build/@wordpress/element/src/serialize.js:31:1)

Test Suites: 3 failed, 3 total
Tests:       0 total
Snapshots:   0 total
Time:        2.949 s
Ran all test suites.

Specifying transformIgnorePatterns in our custom jest config does not fix it either.

Anybody any ideas? Sorry to bother you with this, I assume I missed something..

UPDATE: If I delete node_modules/@wordpress/element/node_modules things start working 🤷‍♂️ I wonder why this is even there in the first place, faulty package build maybe?
UPDATE2: works also if I add transformIgnorePatterns: ['node_modules/@wordpress/element/node_modules/(?!(is-plain-obj))'], I was assuming node_modules/(?!(is-plain-obj)) should be enough already to catch that too hmm

@acicovic
Copy link

@gziolo
Copy link
Member

gziolo commented Aug 16, 2022

@tyxla, I think the regular expression used should match the last occurrence of the node_modules in the path in case we encounter packages installed as deep dependencies like in the case shared above:

node_modules/@wordpress/element/node_modules/is-plain-obj/index.js

We don't care here about the node_modules/@wordpress/element part.

@gziolo gziolo reopened this Aug 16, 2022
Core JS automation moved this from Done to In progress Aug 16, 2022
@tyxla
Copy link
Member

tyxla commented Aug 16, 2022

@gziolo I think this should resolve it: #43271. Do we have a clear case where we can test it to be sure?

@tyxla
Copy link
Member

tyxla commented Aug 16, 2022

@herndlm could you try altering the regex from node_modules/(?!(is-plain-obj)) to node_modules/(?:(?!is-plain-obj/).)*$? and let me know if it works for you? See #43271 where I'm suggesting this. Also cc @acicovic.

@herndlm
Copy link

herndlm commented Aug 16, 2022

@tyxla thank you for working on this. I can indeed confirm that it's working fine with your new regex! to be sure I directly replaced it in node_modules/@wordpress/jest-preset-default/jest-preset.js

@tyxla
Copy link
Member

tyxla commented Aug 16, 2022

Thanks for confirming so quickly, @herndlm!

@gziolo
Copy link
Member

gziolo commented Aug 16, 2022

Excellent! Thank you for providing a fix so quickly. I will publish it to npm tomorrow.

@gziolo
Copy link
Member

gziolo commented Aug 17, 2022

New 8.5.2 version of @wordpress/jest-preset-default is available on npm. I hope it resolves the issue reported.

@gziolo gziolo closed this as completed Aug 17, 2022
Core JS automation moved this from In progress to Done Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Dev Ready for, and needs developer efforts npm Packages Related to npm packages [Package] Jest preset /packages/jest-preset-default [Package] Scripts /packages/scripts [Type] Bug An existing feature does not function as intended
Projects
No open projects
Core JS
  
Done
Development

No branches or pull requests

5 participants