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

TS compilation is broken because of FormData since v10.0.2 #417

Closed
tiger-seo opened this issue Aug 20, 2019 · 18 comments
Closed

TS compilation is broken because of FormData since v10.0.2 #417

tiger-seo opened this issue Aug 20, 2019 · 18 comments

Comments

@tiger-seo
Copy link
Contributor

tiger-seo commented Aug 20, 2019

Description
since version 10.0.2 my build is broken because of FormData, please, see error output below:

> tsc --outDir lib

node_modules/gitlab/dist/infrastructure/index.d.ts:2:8 - error TS1259: Module '"D:/my-project/node_modules/@types/form-data/index"' can only be default-imported using the 'esModuleInterop' flag

2 import FormData from 'form-data';
         ~~~~~~~~

  node_modules/@types/form-data/index.d.ts:15:1
    15 export = FormData;
       ~~~~~~~~~~~~~~~~~~
    This module is declared with using 'export =', and can only be used with a default import when using the 'esModuleInterop' flag.

i guess this is a regression after #401

Expected behaviour
build success

Actual behaviour
build error

@jdalrymple
Copy link
Owner

Hmm, ill give it a look!

@jdalrymple
Copy link
Owner

Not sure why i dont see this error :/

@jetersen
Copy link
Contributor

You need a test project that has a different tsconfig I believe

@jdalrymple
Copy link
Owner

Not sure how to fix it though. Besides removing the type check :s

@jdalrymple
Copy link
Owner

Just removed the reference to @types/form-data since its deprecated anyways. Will test further later

@jdalrymple
Copy link
Owner

https://github.com/jdalrymple/example

@tiger-seo I tried recreating the error here, but im having no luck :( any suggestions?

@tiger-seo
Copy link
Contributor Author

@jdalrymple does it mean i am required to add "esModuleInterop": true in my tsconfig ?

@jdalrymple
Copy link
Owner

I mean you shouldnt have to since the library is just a dependency. Can you see if you can recreate the error using the example repo?

@tiger-seo
Copy link
Contributor Author

@jdalrymple regarding your example, it does compile without error if esModuleInterop is true, but if it is commented then:

>./node_modules/.bin/tsc --outDir dist
src/BaseService.ts:2:8 - error TS1259: Module '"D:/github/example/node_modules/form-data/index"' can only be default-imported using the 'allowSyntheticDefaultImports' flag

2 import FormData from 'form-data';
         ~~~~~~~~

  node_modules/form-data/index.d.ts:10:1
    10 export = FormData;
       ~~~~~~~~~~~~~~~~~~
    This module is declared with using 'export =', and can only be used with a default import when using the 'allowSyntheticDefaultImports' flag.

src/index.ts:1:8 - error TS1259: Module '"D:/github/example/node_modules/form-data/index"' can only be default-imported using the 'allowSyntheticDefaultImports' flag

1 import FormData from 'form-data';
         ~~~~~~~~

  node_modules/form-data/index.d.ts:10:1
    10 export = FormData;
       ~~~~~~~~~~~~~~~~~~
    This module is declared with using 'export =', and can only be used with a default import when using the 'allowSyntheticDefaultImports' flag.


Found 2 errors.

@jdalrymple
Copy link
Owner

Just to verify, @tiger-seo if you set the esModuleInterlop=true in YOUR project, this problem goes away? Trying to narrow it down for the issue in the form-data repo

@dcharbonnier
Copy link

yes @jdalrymple in this case it goes away

@jetersen
Copy link
Contributor

@DanielRose
Copy link
Contributor

@Casz @jdalrymple The problem is different and not due to form-data. form-data is a CommonJS module. It correctly and legally exports the class FormData as its only export, i.e.
module.exports = FormData;

To import the module from another CommonJS module, you would normally use import FormData = require('form-data');

However, trying to use it from an ES6 module isn't that easy and needs a bit of work.
Use Node itself, you would need to use module.createRequire() (see https://nodejs.org/api/esm.html#esm_commonjs_json_and_native_modules).
Alternatively, Babel or Webpack 4 can emit some boilerplate code to make importing work, and the same is done in Typescript with the flag esModuleInterop (see https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-7.html#support-for-import-d-from-cjs-from-commonjs-modules-with---esmoduleinterop)

For these latter cases, you would use form-data as if it had a default export (which Babel/Webpack 4/Typescript will generate), i.e. import FormData from 'form-data'; or written more explicitly: import { default as FormData } from 'form-data';

That way will you import the class, which you then can construct via new FormData(), such as in KyRequester.ts.

It is illegal in ES6 to import the namespace, i.e. import * as FormData from 'form-data'; and then construct it.
Note that this is often done incorrectly for CommonJS modules, ex. import * as express from 'express'; new express();, so (without esModuleInterop) Typescript will silently fix the import. But it is wrong (see the Typescript release notes above).

node-gitlab is written as an ES6 module with esModuleInterop set (see the tsconfig.json).

This is all preamble to understand why node-gitlab has esModuleInterop set.

Anyway, node-gitlab uses rollup to create three variants: CommonJS, ES6, (browser) UMD. All use the other Typescript settings, especially esModuleInterop. This results in the boilerplate code (if necessary) to import form-data as if it had a default export.

The problem is that the generated type definition files also assume that esModuleInterop is set. So the infrastructure/index.d.ts includes the line import FormData from 'form-data';

What happens in this issue is that someone tries to use node-gitlab, but doesn't have esModuleInterop set. So Typescript tries to import the default export from form-data, which doesn't exist (since Typescript didn't generate it). That results in the compile failure.

There are two ways of fixing this:

  1. Everyone who uses node-gitlab will need to set esModuleInterop. Maybe add this to the README and also as a comment in the infrastructure/index.ts (which will be used as source for the index.d.ts)
  2. Since we are only interested in type completion/documentation in the infrastructure/index.ts, change the import to a namespace import, i.e. import * as FormData from 'form-data'; in that file. Due to node-gitlab's having esModuleInterop set, Typescript will fail the compile if it later gets used incorrectly.

@jdalrymple
Copy link
Owner

@DanielRose Thank you! Ill try your suggestion

@jdalrymple
Copy link
Owner

@tiger-seo Give it a try after the latest release is deployed :)

jdalrymple pushed a commit that referenced this issue Aug 30, 2019
## [11.0.2](11.0.1...11.0.2) (2019-08-30)

### Bug Fixes

* Switching type import to hopefully fix [#417](#417) ([91cfbf2](91cfbf2))
@jdalrymple
Copy link
Owner

🎉 This issue has been resolved in version 11.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@tiger-seo
Copy link
Contributor Author

tiger-seo commented Sep 3, 2019

@jdalrymple yes, the issues is fixed, thank you;

@DanielRose thanks for detailed explanation ;)

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

5 participants