"react-scripts test" fails for valid JS files from modules which are successfully built

[dmythro]

Is this a bug report?

Yes

Environment

Environment:
  OS:  macOS High Sierra 10.13.6
  Node:  10.8.0
  Yarn:  1.10.1
  npm:  6.2.0
  Watchman:  4.9.0
  Xcode:  Xcode 10.0 Build version 10A255
  Android Studio:  Not Found

Packages: (wanted => installed)
  react: ~16.5.0 => 16.5.2
  react-dom: ~16.5.0 => 16.5.2
  react-scripts: ^2.0.1 => 2.0.1

Steps to Reproduce

1. Add any module via git (test one: z-ax/test-next-module)
2. Add there a valid JS file which has import something from 'something'
3. Add a simple valid test which uses a file from this module (test one: z-ax/test-next)
4. Run react-scripts build (all good)
5. Run react-scripts test --verbose (test with this import fails)

Expected Behavior

Build passes.
Test passes.

Actual Behavior

Build passes.
Test fails because of SyntaxError: Unexpected identifier, pointing to import.

Linking with #5103.

[bugzpodder]

Can you provide a small repo demonstrating this bug?

[gaearon]

Did this work before with 1.x?
A repro case would indeed be helpful.

[dmythro]

Added test repos as example (updated description):

• CRA@next project: https://github.com/z-ax/test-next
• test module: https://github.com/z-ax/test-next-module

The project starts, builds, renders "43".
But test which imports test module fails with syntax error.

Basically, import works just fine with Jest for project files, but it fails when inside a module, which has to be built too by the new v2 approach, as I think. Inconsistency between scripts.

[bugzpodder]

I can repo with the setup above but this also fails with react-scripts: 1.1.5

test-next/node_modules/test-next-module/index.js:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,global,jest){import FortyTwo from './TestNoop'
                                                                                             ^^^^^^
    
    SyntaxError: Unexpected token import

[dmythro]

I’m not telling it is a regression. It is inconsistency.
V2 has a new feature, build modules, but it doesn’t work for tests the same way, for example when I want to test integration with a module.

[gaearon]

We'll need to think about this.

Compiling newer features does work for tests AFAIK — but just not for import / export syntax. The reason being that we're in a Node environment, and Node will eventually ship a real import / export implementation which might be incompatible with whatever we implement now. So that will become a problem. It's even more complicated because Jest itself has its own custom module system, and that system will also need to support import / export. So there's another layer.

I'm worried that compiling import / export in modules required through their main entrypoint will cause too many problems when/if an incompatible implementation ships in Node and/or Jest. While we only compile import/export inside the app itself, the damage is limited.

Another way to look at it is: if you ship import / export in your package today your code already doesn't work in Node, and will probably stay broken in Node even after it supports ESM. Jest is a tool for Node environment, so I think it's fair to make an assumption that any published package you attempt to import there should have a decent chance of working in Node.

Now, I know your use case is a bit different: submodule etc. It's not really third-party code you plan to publish. But then the problem you have is more about #5161, which is a missing feature. Even if we solved this from import / export you'd still have that issue for JSX etc.

[Timer]

Yeah, considering this was the prior behavior in v1, I'd said it'd be appropriate to merge this with #5161.

[dmythro]

@gaearon @Timer I agree with your points, let's do it this way and merge with #5161 as it is more like a sub-set of the proposal (and as I see a pretty rare case for now at least because the main feature is new).

So, I'm closing this one and adding a brief and reference into #5161 description.