Skip to content
This repository has been archived by the owner on Jan 26, 2019. It is now read-only.

Update tsconfig to support dynamic import #115

Merged
merged 4 commits into from
Jul 27, 2017
Merged

Conversation

nicolaserny
Copy link
Contributor

resolves #90

In order to make code splitting work, we have to set the module to esnext in tsconfig.json

@nicolaserny
Copy link
Contributor Author

@wmonk using esnext seems to have an impact on tests (SyntaxError: Unexpected token import).
The weird part is that tests (yarn test) work on my projects created with create react app ts + changing module to esnext.
I'm not sure it's a good idea to override the tsconfig file used by jest...

@nicolaserny
Copy link
Contributor Author

@wmonk only a temporary fix (for jest) because TS_CONFIG is deprecated
https://github.com/kulshekhar/ts-jest
as we provide a tsConfigFile value, the module is not overwritten to commonjs...

@DorianGrey
Copy link
Collaborator

What about creating a tsconfig.test.json which uses the extends directive and overrides the module field along with the regular tsconfig.json, and update the reference to tsConfigFile accordingly? Path may be added like the current one that references the regular tsconfig.json.

I.e.:
tsconfig.test.json

{
  "extends": "./tsconfig.json",
  "compilerOptions": {
    "module": "commonjs"
  }
}

Add corresponding path entries in this file, e.g. appTsTestConfig (keep in mind that there are multiple positions to be extended, i.e. L84 and L114.
As a last step, change the variable here to your new one.

It's a bit tricky, but different module values are required under different circumstances:

  • esnext for the dynamic import call
  • commonjs to get things working on node.js

Using different config files for both situations should be the way to go here.

Copy link
Collaborator

@DorianGrey DorianGrey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wmonk:
Change looks good technically. Your turn now :)

@wmonk wmonk merged commit a445a40 into wmonk:master Jul 27, 2017
@wmonk
Copy link
Owner

wmonk commented Jul 27, 2017

Thanks!

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

Successfully merging this pull request may close these issues.

Dynamic import not working
3 participants