Fix: 'Cannot use import statement outside a module' error in generated extensible project unit tests @W-15705195@

[joeluong-sfcc]

Description

There is currently an issue (#1763) in generated projects with extensibility where if you try to run unit tests, you can end up with the following error:

Jest encountered an unexpected token

...

Details:

    /retail-react-app-demo/node_modules/@salesforce/retail-react-app/app/components/_app-config/index.jsx:16

import React from 'react'
    ^^^^^^

    SyntaxError: Cannot use import statement outside a module

      1 | import React from 'react'
      2 | import {render, waitFor} from '@testing-library/react'
    > 3 | import AppConfig from '@salesforce/retail-react-app/app/components/_app-config/index.jsx'
        | ^

Note that the SyntaxError: Cannot use import statement outside a module error comes from within the node_modules, specifically at node_modules/@salesforce/retail-react-app/app/components/_app-config/index.jsx

This issue occurs because transformations are not applied to node_modules by default. The files within the node_modules/@salesforce/retail-react-app directory are not formatted in the way that Jest expects. The fix is to ensure transformations are applied to node_modules/@salesforce/retail-react-app by configuring it in the jest.config.js file.

Steps to reproduce:

1. Generate project: npx @salesforce/pwa-kit-create-app@latest --preset=retail-react-app-demo
2. cd retail-react-app-demo
3. Add a unit test (index.test.js) to retail-react-app-demo/overrides/app/components/_app-config/

// Import a variety of things from the node_modules
import React from 'react'
import {render, waitFor} from '@testing-library/react'

import AppConfig from '@salesforce/retail-react-app/app/components/_app-config/index.jsx'

import {CorrelationIdProvider} from '@salesforce/pwa-kit-react-sdk/ssr/universal/contexts'
import {uuidv4} from '@salesforce/pwa-kit-react-sdk/utils/uuidv4.client'
import {StaticRouter} from 'react-router-dom'

import mockConfig from '@salesforce/retail-react-app/config/mocks/default'
import {rest} from 'msw'
import {registerUserToken} from '@salesforce/retail-react-app/app/utils/test-utils'

test('Sample unit test', () => {
    expect(true).toBe(true)
})

4. npm run test
5. Observe failure

Types of Changes

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Documentation update
• Breaking change (could cause existing functionality to not work as expected)
• Other changes (non-breaking changes that does not fit any of the above)

Breaking changes include:

• Removing a public function or component or prop
• Adding a required argument to a function
• Changing the data type of a function parameter or return value
• Adding a new peer dependency to package.json

Changes

• Add jest.config.js file to generated projects with extensibility enabled

How to Test-Drive This PR

1. git checkout ju/fix-import-extensible-tests
2. From the root directory: node ./packages/pwa-kit-create-app/scripts/create-mobify-app.js --preset=retail-react-app-demo
3. cd retail-react-app-demo
4. Add sample unit test from above (index.test.js) to retail-react-app-demo/overrides/app/components/_app-config/
5. npm run test
6. Observe no failures

Checklists

General

• Changes are covered by test cases
• CHANGELOG.md updated with a short description of changes (not required for documentation updates)

Accessibility Compliance

You must check off all items in one of the follow two lists:

• There are no changes to UI

or...

• Changes were tested with a Screen Reader (iOS VoiceOver or Android Talkback) and had no issues
• Changes comply with WCAG 2.0 guidelines levels A and AA
• Changes to common UI patterns and interactions comply with WAI-ARIA best practices

Localization

• Changes include a UI text update in the Retail React App (which requires translation)

[bendvc]

@joeluong-sfcc I think we need to think about the bigger picture here, does it make sense for us to allow end-users the ability to run tests for projects that we are extending from? Have we thought about ignoring running any tests that are in the extended project as a solution here?

[joeluong-sfcc]

@joeluong-sfcc I think we need to think about the bigger picture here, does it make sense for us to allow end-users the ability to run tests for projects that we are extending from? Have we thought about ignoring running any tests that are in the extended project as a solution here?

@bendvc You bring up a good point. I think that certain tests wouldn't really make sense in an extended project, like tests that retest the implementation of a component from the underlying project. However, I still think we should allow end users to write unit tests in their extended project using exported components/functions from the underlying project. For example, I can imagine a scenario where a user would want to test the behavior of their overriding component and compare it to the underlying component that they're overriding.

I'd like to get your thoughts as well, do you think it makes sense to allow users to write tests in this manner (or other tests in general) in their extended projects?

[bendvc]

@joeluong-sfcc I think we need to think about the bigger picture here, does it make sense for us to allow end-users the ability to run tests for projects that we are extending from? Have we thought about ignoring running any tests that are in the extended project as a solution here?

@bendvc You bring up a good point. I think that certain tests wouldn't really make sense in an extended project, like tests that retest the implementation of a component from the underlying project. However, I still think we should allow end users to write unit tests in their extended project using exported components/functions from the underlying project. For example, I can imagine a scenario where a user would want to test the behavior of their overriding component and compare it to the underlying component that they're overriding.

I'd like to get your thoughts as well, do you think it makes sense to allow users to write tests in this manner (or other tests in general) in their extended projects?

Yes. I agree that if we have a "test" script it implies that we have some kind of testing framework setup and executing that script will run it. So if a developer that is working on an extended project wants to add tests for a new component, whether that component is a new page or an override, it should be executed when they use npm run test. But what shouldn't happen is the entire set of tests being run for the underlaying template that is being extended.

I'm open to discussing reasons why we might want to do that, but I can't think of any off the top of my head. The template being extended is, kind of a "grey" box, we know the api and how to use it, but we shouldn't be running the tests we created during an extended projects build cycle.

[kevinxh]

it's more important to have the "why"s than "how"s in code comment.

Suggested change

	// Don't ignore @salesforce/retail-react-app
	// To support extensibility, jest needs to transform the underlying templates/extensions

[kevinxh]

what happens if the project wants to extend from another project or have multiple extensions

if the package.json looks like

{
  ccExtensibility: ['@salesforce/retail-react-app', 'myCustomExtensionA']
}

Does that mean they'll need to update this jest config to include all extensions? How can we build an API that does that for the users? 🤔

[kevinxh]

for example, can the base jest.config.js be smart enough to automatically build transformIgnorePatterns that includes all extensions?

[joeluong-sfcc]

From what I understand, all the extensions will live under the @salesforce/extensions directory in the node_modules, so when we implement that change we can just update the jest config to transform the files in that directory

[kevinxh]

does that mean we'll run babel to transpile all node_modules dependencies? I wonder if that has negative impact on performance, do you want to give it a try and see if it works?

[joeluong-sfcc]

No, if we wanted to transform all the dependencies inside node_modules, we would set transformIgnorePatterns to an empty array

module.exports = {
    ...base,
    // this means ignore nothing, meaning transform everything, including node_modules
    transformIgnorePatterns: [],
    moduleNameMapper: {
        ...base.moduleNameMapper,
        '^is-what$': '<rootDir>/node_modules/is-what/dist/cjs/index.cjs',
        '^copy-anything$': '<rootDir>/node_modules/copy-anything/dist/cjs/index.cjs'
    }
}

Here are the performance results of how long it takes to run npm run test in the demo project:

Transform only @salesforce/retail-react-app (how it is currently set up)

• First time: 6.158 s
• Second time: 2.899 s

Transform all node modules

• First time: 28.537 s
• Second time: 3.973 s

[kevinxh]

thanks for trying it out, yea i think we definitely don't want to transform all node modules, it's too slow.