create-react-app-typescript icon indicating copy to clipboard operation
create-react-app-typescript copied to clipboard

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

Open eonwhite opened this issue 6 years ago • 26 comments

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

[email protected] 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.

eonwhite avatar Jul 31 '17 01:07 eonwhite

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: https://github.com/Microsoft/TypeScript/issues/8760#issuecomment-221025897

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.

DorianGrey avatar Jul 31 '17 07:07 DorianGrey

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 https://github.com/facebookincubator/create-react-app/issues/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.

eonwhite avatar Jul 31 '17 13:07 eonwhite

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: https://github.com/kulshekhar/ts-jest/issues/112 However, ts-jest uses transpileModule, which should work properly here. There has been a problem with this in the past (see https://github.com/Microsoft/TypeScript/issues/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 avatar Jul 31 '17 14:07 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 ?

DorianGrey avatar Aug 01 '17 06:08 DorianGrey

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 https://github.com/facebookincubator/create-react-app/issues/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.

eonwhite avatar Aug 01 '17 16:08 eonwhite

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.

DorianGrey avatar Aug 02 '17 06:08 DorianGrey

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?

wmonk avatar Aug 08 '17 11:08 wmonk

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.

DorianGrey avatar Aug 08 '17 12:08 DorianGrey

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?

eonwhite avatar Aug 08 '17 17:08 eonwhite

i'd come across this issue when wanting to use absolute paths in my code for helper methods, such as in <% APP_ROOT %>/src/utils/http.ts for instance.

instead of having to do ../../../../../utils/http, i'd wanted it to be at least src/utils/http

i achieved this in 4 steps:

  1. npm install --save-dev react-app-rewired
  2. updating tsconfig.json:
{
  "compilerOptions": {
    "baseUrl": ".",
    "paths": {
      "src/*": ["src/*"]
    },
    ...
}
  1. adding a config-overrides.js file into the base of the project:
const path = require('path');

module.exports = {
  webpack: (config, env) => {
    config.resolve.alias.src = path.resolve(__dirname, './src/');
    return config;
  },
  jest: (config) => {
    config.modulePaths = ['src/'];
    return config;
  }
}
  1. updating package.json:
"scripts": {
  "start": "react-app-rewired start --scripts-version react-scripts-ts",
  "build": "react-app-rewired build --scripts-version react-scripts-ts",
  "test": "react-app-rewired test --scripts-version react-scripts-ts --env=jsdom",
  ...
}

To get tests working (jest), in package.json:

  "jest": {
    "moduleNameMapper": {
      "src/(.*)": "<rootDir>/src/$1"
    },
    "moduleDirectories": [
      "node_modules",
      "src"
    ]
  },

comments, questions and suggestions welcome to improve this. otherwise, i hope this helps for anybody else running into this issue

imzaid avatar Jan 09 '18 19:01 imzaid

@imzaid If I do that I get the following error when running npm test:

Out of the box, Create React App only supports overriding these Jest options:

  • collectCoverageFrom
  • coverageReporters
  • coverageThreshold
  • snapshotSerializers.

These options in your package.json Jest configuration are not currently supported by Create React App:

  • moduleDirectories
  • moduleNameMapper

If you wish to override other Jest options, you need to eject from the default setup. You can do so by running npm run eject but remember that thisis a one-way operation. You may also file an issue with Create React App to discuss supporting more options out of the box.

bjarkehs avatar Jan 27 '18 12:01 bjarkehs

hi @bjarkehs, sorry about that. it seems i missed updating the test portion in package.json:

"scripts": {
  ...
  "test": "react-app-rewired test --scripts-version react-scripts-ts --env=jsdom",
  ...
}

i've updated my previous post to reflect that. let me know if that ends up working out for you!

imzaid avatar Jan 27 '18 23:01 imzaid

I am currently trying to get absolute imports working with ~ as an alias to <rootDir>/src. For now I just wanted to say that one of the solutions that's been proposed here -- using jest's modulePaths to set up the alias -- would not work for this use case, because I don't actually have a folder called ~. So IMO a more flexible solution would be better even if it's "hackier" as @DorianGrey said. Using a tilde is a fairly common convention, and I don't think there's any way to set it up without using the moduleNameMapper option.

mbrowne avatar Mar 26 '18 20:03 mbrowne

@mbrowne,

i'm not sure that's wise as you're assuming that others working on your project will also have to have the same resources in their home directory (~) as you do to run your tests. ideally you'd want everything self contained in your project itself in which case <rootDir> will be sufficient enough.

if you're the only one working on your project and you insist on using ~, then the hackier approach probably will suit you best

imzaid avatar Mar 26 '18 21:03 imzaid

That's not what I'm doing..here's some code to help explain. I have this in my tsconfig.json:

"baseUrl": ".", // all paths are relative to the baseUrl
"paths": {
  "~/*": ["src/*"] // resolve any `~/foo/bar` to `<baseUrl>/src/foo/bar`
},

This works great everywhere except for when trying to run jest. So I ejected the config and added this to the "jest" section of package.json:

"moduleNameMapper": {
  "~/(.+)": "<rootDir>/src/$1",

To be clear, I'm not referring to anyone's home folder on their computer; ~ is simply a short and convenient alias to the src folder. At first I thought it wasn't working, but I have since figured it out...it works fine, I was just encountering an unrelated issue when jest tries to resolve import paths in node_modules containing a slash (I'll comment separately about that). The ~ convention is used in some other JS projects as well, for example https://nuxtjs.org/ (yes I realize that's not a React lib, just mentioning it as an example of the ~ convention).

Anyway, regardless of how people feel about using ~ as an alias, my point is that it would be nice to be able to use the "moduleNameMapper" option without ejecting. People might also want custom aliases to deeply nested folders, for example.

mbrowne avatar Mar 26 '18 23:03 mbrowne

Unrelated to this thread (other than the same issue of Jest not compiling code successfully)... In case anyone is curious, the issue I referred to above turned out not to be related to slashes in import paths but rather .mjs files in node_modules. (I was confused because I first encountered the issue due to this line in my app: import withApollo, { WithApolloClient } from 'react-apollo/withApollo.) See #287

mbrowne avatar Mar 27 '18 20:03 mbrowne

I'm running into this problem because I wrote my own library that exports ES6 style modules that I refer to directly in my create-react-native-typescript app, like import { Stuff } from "myLibrary/dist/stuff".

Since "myLibrary/dist/stuff.js" is still using ES6-style exports, when I run tests they blow up. Should I be compiling down the exports in those files before publishing my library (even though I'm the only one using it, and exclusively in projects that support ES6 modules), and if I did would it impact my frontend app's ability to tree shake that code?

osdiab avatar Jun 13 '18 22:06 osdiab

@osdiab you could try using .mjs files, which support ES6 modules out of the box. The previous issue with using .mjs files in create-react-app (and create-react-app-typescript -- whether in your app code or in libraries) has been resolved. But this is just an idea; I haven't tried it and don't know if it will actually work with jest. It seems there are still some issues with jest and .mjs files: https://github.com/facebook/jest/issues/4637

An easier short-term solution might be to tell jest to ignore your library, e.g. using this in package.json (assuming your library is in node_modules):

"transformIgnorePatterns": ["[/\\\\]node_modules[/\\\\](?!(my-library)/)"],

(The latter solution is how I was able to get jest working with react-apollo.)

mbrowne avatar Jun 14 '18 01:06 mbrowne

hmmm i didn't want to eject so i added the transformIgnorePatterns to the invocation of react-scripts-ts jest; it almost works but now I'm getting a separate problem, let me know if this is too off topic.

An import of a separate library, import * as urlJoin from 'url-join'); urlJoin(a, b, c) is failing because rather than being imported as a old school node module (it doens't use es6 syntax), it's getting imported in the test as a module with a default export (urlJoin.default is a function, urlJoin is an object).

Why would that happen? I don't really know what the jest typescript transform under the hood is doing but everywhere else it seems fine to run import * as urlJoin as 'url-join' except when that test runs. So confused lol

osdiab avatar Jun 14 '18 02:06 osdiab

Do you have esModuleInterop set to true in tsconfig.json? In any case see if changing that setting makes a difference.

mbrowne avatar Jun 14 '18 02:06 mbrowne

I had it false, but turning it on caused a bunch of other errors in my project so I’d rather not. Eh maybe I’ll just eject :P

osdiab avatar Jun 14 '18 04:06 osdiab

In tsconfig.json Changed: "module": "es6" ---> "module": "commonjs"

mihaa1 avatar Jun 26 '18 06:06 mihaa1

Having this issue on my vuejs app. Making this configuration change under "jest" in my package.json fixed it.

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

rahulakurati avatar Jul 05 '18 18:07 rahulakurati

@lopno I got it working using the following:

in your tsconfig.json add:

"baseUrl": ".",
"paths": {
  "src/*": ["src/*"]
}

And in your package.json change to the following:

"scripts": {
  "start": "NODE_PATH=./ react-scripts-ts start",
  "build": "NODE_PATH=./ react-scripts-ts build",
  "test": "NODE_PATH=./ react-scripts-ts test --env=jsdom"
},

Notice the NODE_PATH=./. This will fix jest. However, please note that this only works when your source folder is called src and your absolute imports are of the format:

import Foo from 'src/components/Foo'`

Hope this helps.

bjarkehs avatar Aug 22 '18 13:08 bjarkehs

@bjarkehs Hej Bjarke, long time no see.

I got it working doing this:

In tsconfig.json

"paths": {
  "src/*": ["src/*"]
}

And adding a jest config in package.json

"jest": {
    "moduleNameMapper": {
    "^src/(.*)$": "<rootDir>/src/$1"
    }
  },

lopno avatar Aug 24 '18 13:08 lopno

"compilerOptions": {
    "baseUrl": ".",
    "paths": {
      "*": [
        "node_modules/*",
        "src/*"
      ]
    },
    ...
}

This is working for me.

bashjs avatar Oct 31 '18 00:10 bashjs