jest tests failing with "Unexpected token export" when using absolute imports

[eonwhite]

Description

The apparent recommended solution for achieving absolute imports, most recently referenced in #119, is to add a node_modules folder or symlink in src. Because NODE_PATH doesn't work in Typescript, I believe this is the only way to do absolute imports in create-react-app-typescript.

However, I am seeing jest tests choke when using such imports with SyntaxError: Unexpected token export errors.

Weirdly this may be somehow related to using enums? See the reproducible demo below.

Expected behavior

Expect jest tests to work.

Actual behavior

Jest fails with:

    SyntaxError: Unexpected token export

      at ScriptTransformer._transformAndBuildScript (node_modules/jest-runtime/build/ScriptTransformer.js:289:17)
      at Object.<anonymous> (src/App.tsx:15:17)
      at Object.<anonymous> (src/App.test.tsx:5:13)

Environment

react-scripts-ts@2.5.0
node 8.2.1
npm 5.3.0

Reproducible Demo

I created a reproducible demo here:

https://github.com/eonwhite/absolute-imports

To reproduce the issue, git clone and npm install, then:

• Try npm start and note that the "app" works.
• Try npm test and note that tests fail with SyntaxError: Unexpected token export

Note that you can get jest to run and the tests to pass by:

• changing the absolute import on line 5 of App.tsx to a relative import: import { MyModel, Type } from './common/model/MyModel';
• removing the Type enum in src/common/model/MyModel.ts (and the references to the enum in App.tsx)

Obviously though I'm seeking a solution that lets me use absolute imports and enums in my code.

[DorianGrey]

This "recommendation" was never made - it's an assumption of the user that closed the issue without waiting for a response. And it's just a hack that isn't proven to work in general.

The error message you get indicates that the file was never processed by the typescript compiler, and thus still contains the export clause.
As you recognized correctly, typescript does not support "NODE_PATH" (using it is discouraged in general, from what I know) - instead, you have to go with path mapping, as suggested here: microsoft/TypeScript#8760 (comment)

However... this is not as easy is it seems to be, at least considering the technical circumstances. These path mappings have to be added to:

• the tsconfig.json
• the webpack configuration
• the jest configuration
  The first one is the easiest one. If you change your compiler options to something like this (removed rootDir, added baseUrl and paths), you may compile your code using node_modules/.bin/tsc without a problem:

"compilerOptions": {
    "outDir": "build/dist",
    "module": "esnext",
    "target": "es5",
    "lib": ["es6", "dom"],
    "sourceMap": true,
    "allowJs": true,
    "jsx": "react",
    "moduleResolution": "node",
    "forceConsistentCasingInFileNames": true,
    "noImplicitReturns": true,
    "noImplicitThis": true,
    "noImplicitAny": true,
    "strictNullChecks": true,
    "suppressImplicitAnyIndexErrors": true,
    "noUnusedLocals": true,
    "baseUrl": ".",
    "paths": {
      "model/*": ["src/common/model/*"]
    }
}

And that's where the problem with the default CRA setup begins - without ejecting the project, you won't have access to the other configs to add an entry to resolve.alias in the webpack config, and moduleNameMapper to the jest config.

While this is possible ... I wouldn't recommend it. It will force you to maintain three definition points for the path mappings, each of them with a different syntax (TS: Glob, Webpack: Plain path, Jest: Regexp). I'd recommend to deleted your src/node_modules dir/link and use relative paths. I've tried that with you demo, and it works without a problem. In that case, you won't even need to touch the tsconfig yourself.
However, if you consider to have great benefits from using absolute paths ... you have to go with ejection.

[eonwhite]

Thanks for the detailed response.

To clarify, the recommendation to use node_modules was originally made by Dan Abramov (CRA author) in the main CRA project facebook/create-react-app#1065, so doing that way is a semi-official recommendation, at least for the main project.

And src/node_modules seems to work pretty well, especially with tooling. I have two mid-sized projects that work fine with this approach, until I ran into this jest issue with enums.

The fact that introducing the enums causes the issue, gives me some kind of hope this might be a fixable bug in this project. The tsc compiler is obviously processing the MyModel.ts file somehow in jest tests, or the export interface MyModel without the enum wouldn't work. It must be doing some kind of separate pass for enum parsing, that's getting missed. I don't know tsc compilation well enough to have a clue what's going on.

I'll experiment with ejection config, but it seems like a real shame if create-react-app-typescript has no official way to support absolute imports without ejection. create-react-app has both the node_modules approach and NODE_PATH, neither of which require ejection.

[DorianGrey]

And both solutions are not ideal, technically - although they work fine in quite a lot of cases, these are more like workarounds.

Regarding the enum issue - I've found some issues in the ts-jest (which is used for transformation) repo about this problem as well:
kulshekhar/ts-jest#112
However, ts-jest uses transpileModule, which should work properly here. There has been a problem with this in the past (see microsoft/TypeScript#5243), but that should not have a negative impact in this case.
I have to admit that I'm a bit confused why this problem only occurs when using absolute imports with ts-jest, and not in any other case (using tsc directly or using relative imports works fine). I'll see if I can dig a little deeper into this to figure out what causes this difference.

[DorianGrey]

Ok, got it - the problem is caused by this line:

transformIgnorePatterns: [
  '[/\\\\]node_modules[/\\\\].+\\.(js|jsx|ts|tsx)$',
]

When changing it to

transformIgnorePatterns: ["[/\\\\]node_modules[/\\\\].+\\.(js|jsx)$"]

(like in the original repo) everything works fine.

A bit more detailed: As I considered when reading the original error message, the MyModel.ts file did not get processed by the preprocessor. Thus, when jest picked it up when dealing with App.tsx, it complained about the unknown and unexpected expressions.
I did work before you added the enum because the file only contained an interface - these are just types. It contained data only required and used for transpilation, so it was omitted completely afterwards. When adding the enum, the file started to contain data which is present even after transpilation. However, the pattern above forces jest to not put any file through the transformation process that is somewhere below node_modules and has a file extension of js, jsx, ts or tsx. This is intended, since jest might consider to transform files from libraries otherwise, which might have an extremely negative performance impact. But since you sym-linked your model package to be below a node_modules w.r.t. the path, it also gets ignored by the transformer - and that's why your test fails.
Technically, this problem is not tied to enums - it might have been caused by every export that contains data, thus making the file present even after transpilation.
Things work fine when using the start or build command since these are processed by webpack, which uses different loader rules that adhere to include instead of exclude to avoid processing the node_modules folder.
You see, that's why I consider this "solution" to be more of a hacky workaround...

Regarding the change I've tested above - that tends to the same direction. If the exclusion is changed to allow ts and tsx files in node_modules, it might cause that such files are processed even from libraries (quite a lot a publishing their source along with the build). If it remains in its current state, it will break the sym-link workaround for absolute paths, forcing anyone who wants to use them to eject their projects and set up path mapping manually.
It's hard to decide which option is worse, since both are quite unlikely...

@wmonk , what do you think ?

[eonwhite]

Thank you for the detailed investigation!

A few thoughts while waiting for @wmonk to chime in.

(1) I agree that the src/node_modules approach is a bit of a hack. I guess what I'm really looking for is, can we create an official create-react-app-typescript approach to achieving absolute imports? I would prefer to go with whatever the project consensus is and avoid ejecting. (On review it's not even clear that the src/node_modules approach is the consensus in the parent CRA project -- it seems like most people use NODE_PATH.) So I'm totally fine moving away from src/node_modules if it feels too hacky to support. I just want some way to achieve this that doesn't require ejection.

(2) If we do want to support src/node_modules as the way to go (or the least bad way to go), then maybe the immediate fix to this bug is to modify transformIgnorePatterns to specifically allow src/node_modules but ignore any other node_modules? I did a little test and it initially seems to work. Happy to submit a PR if this is the route you'd want to go.

(3) Or perhaps it would be less hacky to specifically designate a folder like common or packages which is officially supported by the project as a place for common imports. (Something like this was suggested here facebook/create-react-app#741 but I guess the most recent consensus per the final comment is to use NODE_PATH).

@jaredpalmer and I created a fork here (https://github.com/palmerhq/create-react-app-typescript/tree/palmerhq) based on your suggestions that sets up common as a base absolute-import path used by TS, Webpack, and Jest. We made the fork for our own use for now, but maybe some variation on this approach could be mergeable into the project?

(4) If the project POV is that absolute imports are simply unsupported (which is totally your right), loads of ../../../.. in import statements is just not something I can live with in a big project where I frequently refactor. So I'll run on the fork for now. But if that's the decision, then probably worth stating it in the docs.

[DorianGrey]

Just a few notes:

1. I agree that this should be possible somehow, esp. since the NODE_PATH approach does not and (apparently) will never work with TS (for understandable reasons). However, the non-hacky technical solutions will only work in ejected mode, so these are out of reach.
2. Explicitly not matching src/node_modules would require a negative lookahead in the regexp - I'm not sure if that works, at least I did not get this working yet. Suppose it would be easier to change the expression so that it only matches the root's node_modules directory:
   <rootDir>[/\\\\]node_modules[/\\\\].+\\.(js|jsx|ts|tsx)$
   Should be a simple change without that much risk of breaking something. The additional runtime overhead of this more complex statement should be negligible or at least acceptable.
3. In that case, we should put bets on the time it takes until the first Oh no, I don't like that directory name, what about changing it to <whatever-maybe-unicorn>? issues are opened ... ;) Would be a clean approach from the technical perspective. However, this is a bit more complicated to achieve, since a change like this would have to support both the default and ejected mode.

I.e.: (2) would be a quick solution, but would enforce the src/node_modules approach. (3) is a (technically) cleaner solution, but requires a bit more testing to ensure both modes are supported without problems. Both approaches shouldn't cause problems in any IDE or editor.

[wmonk]

Thanks both for this discussion! I've read through most of it and am happy to make this work for people, as it seems to be something that comes up relatively often in our issues.

Can you give your opinion on the most effective, but also least likely to impact users who are not interested in this feature way to achieve this?

[DorianGrey]

The most effective - though not technically likely - solution would be to adjust the regexp here to

<rootDir>[/\\\\]node_modules[/\\\\].+\\.(js|jsx|ts|tsx)$

so that it only covers the root's node_modules folder. This only has a chance to break in case someone is still using npm < 3 (introduced the flat dependency hierarchy), and I doubt anyone does. There might be some curious condition that may cause nested node_modules folders (in case of colliding version requirements), but it should not happen considerably often. Only real downside is that it enforces the src/node_modules approach. Quite a lot of people already know about and head this way (if not using NODE_PATH).

The alternative to define a particular folder for absolute imports is not only a more technically advanced topic (since it's required to support both regular and ejected mode), it is also the more opinionated one, as it enforces a particular folder to refer to. Would be the less hacky approach, however. If correctly set up, it should not break anything. It is already illustrated in the fork mentioned above, though it has to checked if it works properly with and without ejection.

Personally, I'd favor the second approach, since I don't like the hacky nature of the first one.

[eonwhite]

I would also vote for the second approach, if we are okay with the project being a little opinionated. I've seen a few other flaky behaviors relating to the use of src/node_modules, so would be happy to move away from it.

I personally use src/common as the folder of choice (and that is the folder used in the fork), but open to whatever.

What would be the next step to move this forward? Should we test the fork and submit a PR?