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

Not processing correctly when using es2015 modules #121

Closed
alexjoverm opened this issue Mar 6, 2017 · 13 comments
Closed

Not processing correctly when using es2015 modules #121

alexjoverm opened this issue Mar 6, 2017 · 13 comments

Comments

@alexjoverm
Copy link
Contributor

alexjoverm commented Mar 6, 2017

Relates to #99

When using es2015 modules (necessary for example for tree-shaking), ts-jest throws an error not recognizing import statement. Such configuration can be:

{
    "compilerOptions": {
        "moduleResolution": "node",
        "target": "es6",
...

But when you use commonjs modules, it works

{
    "compilerOptions": {
        "module": "commonjs"
        "moduleResolution": "node",
        "target": "es6",
...

How to reproduce

git clone -b ts-jest-issue https://github.com/alexjoverm/typescript-library-starter.git
cd typescript-library-starter

npm install # give it the name "library" when asked
npm t

It will throw a SyntaxError: Unexpected token import

Now go to tsconfig.json and add "module": "commonjs" to the compilerOptions property.
Execute npm t -- --no-cache

Then it works

@kulshekhar
Copy link
Owner

I followed your instructions and npm t executed successfully.

What version of node are you using?

@alexjoverm
Copy link
Contributor Author

Sorry, forgot to update the actual test.

Can you try again? Just update library.test.js with:

import { DummyClass } from '../src/library'

/**
 * Dummy test
 */
describe('Dummy test', () => {
  it('works if true is truthy', () => {
    expect(true).toBeTruthy()
  })

  it('DummyClass is instantiable', () => {
    expect(new DummyClass()).toBeInstanceOf(DummyClass)
  })
})

@kulshekhar
Copy link
Owner

kulshekhar commented Mar 6, 2017

try adding the following in the jest block in your package.json:

    "globals": {
      "__TS_CONFIG__": {
        "module": "commonjs"
      }
    }

and change

export default class DummyClass {

}

to

export class DummyClass {

}

@alexjoverm
Copy link
Contributor Author

Yep, I fixed it by adding the first thing. The export default class DummyClass just works fine. But still I thought this is an issue. That solution looks more like a workaround than a fix IMO

@kulshekhar
Copy link
Owner

It's not a workaround.

The output of ts-jest is fed to jest for further processing and jest doesn't understand import. There's nothing else we can do in the preprocessor.

@alexjoverm
Copy link
Contributor Author

Ok, understood. I think I've done it in jest but you need babel-jest and all that stuff if I remember correctly.

If that's the case, probably is better to add it to https://github.com/kulshekhar/ts-jest#known-limitations-for-ts-compiler-options?? Then other people don't stumble into the same issue

@kulshekhar
Copy link
Owner

kulshekhar commented Mar 6, 2017

I can understand adding babel in the final compilation (for production) stage but all adding babel in ts-jest will do is slow down testing. I really don't see any need to add babel here. I'm not sure what the drawback of using commonjs as module (only during testing) is.

This isn't really a limitation of the ts-jest or of typescript. Either will happily process any valid module type. The error was thrown by jest.

I agree about the possibility of confusion here. What could be done is prevent external settings from overriding the module setting during testing and hard-coding it to commonjs. I'll have to think that through though.

btw, you can close this if your issue has been solved :)

@alexjoverm
Copy link
Contributor Author

Hey @kulshekhar, sorry I've been away some days.

I didn't mean using Babel, I was just saying, forget about that :P

IMO, it is a responsability of ts-jest. I mean, the error is triggered from jest, but is caused by the compilation from TS to JS.

For me, 2 possible solutions:

  • What you said sounds good, although I don't know if that could break something. I assume you know that better :)
  • At least, mentioning it in the README would be enough for me. Something that describes, that you have to use module: commonjs or if you use es2015 modules you have to add the __TS_CONFIG__ thing.

What do you think?

@alexjoverm
Copy link
Contributor Author

I could help here if you need btw

@kulshekhar
Copy link
Owner

@alexjoverm The way I see it, what happens between ts-jest and jest shouldn't be the user's concern. So while currently they can set the module, ideally they should have to do nothing beyond indicating the preprocessor & patterns.

I can't think of a drawback of hard-coding commonjs in there. We're currently setting this if there's no value set anyway. Might as well remove the source of confusion.

If done this way, we'll need to add a note in the Options section in the readme that the value for module is set to commonjs during testing.

This should be a simple PR if you choose to take it on :)

@alexjoverm
Copy link
Contributor Author

I agree with your point of view. Sure, I'll take a look this weekend and go for it. Should we re-open the issue then?

@kulshekhar
Copy link
Owner

I've added another issue to track this - #122

@piotrwitek
Copy link

@kulshekhar @alexjoverm isn't this commit introduced a regression to related PR #123?
d26dbf6#diff-e1495d267619047a7cca5cfe8f692729L85

I have the same issue as OP, from what I understood the change was suppose to always override module entry, no matter if tsconfig is loaded from root folder or other folder like src/ in my case.
I think the change from the commit changed to work only for tsconfig loaded from root folder.
This change has broken my test config after the update.

My config:

// jest
{
  "moduleFileExtensions": [
    "ts",
    "tsx",
    "js"
  ],
  "transform": {
    ".(ts|tsx)": "<rootDir>/node_modules/ts-jest/preprocessor.js"
  },
  "globals": {
    "__TS_CONFIG__": "<rootDir>/src/tsconfig.json"
  }
}

//tsconfig
{
  "compilerOptions": {
    "target": "es5",
    "module": "es2015",
    "moduleResolution": "node",
    ...
  }
}

I have created a new issue to track this: #161

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants