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

Import in typescript #16

Closed
ishpartko opened this issue Apr 17, 2020 · 5 comments · Fixed by #17
Closed

Import in typescript #16

ishpartko opened this issue Apr 17, 2020 · 5 comments · Fixed by #17
Assignees

Comments

@ishpartko
Copy link

ishpartko commented Apr 17, 2020

Hi all,

Don't working default import in ts, like import ky from 'ky-universal' is undefiend
but:

const ky = require('ky-universal')

is working

@sholladay
Copy link
Collaborator

sholladay commented Apr 18, 2020

I believe this is to be expected. Unlike ky, which is a pure ES module, ky-universal is a CommonJS module. I think Sindre did it this way because CommonJS is still a little easier to consume in the Node.js ecosystem.

By the way, it is probably better to write:

import ky = require('ky-universal');

My understanding is that const ... require() returns the module with type any, whereas import ... require() returns the correct type.

@ishpartko
Copy link
Author

Hi @sholladay,

Thank you for quick response.

import ky = require('ky-universal')

This way shows me only "ky.default.post" path in hints in vscode and works incorrectly because ky-universal exported object that doesn't have the "default" field and app crashed in runtime. If I don't use the "default" field it shows error with compiling and linting in TS.

My solution is add the definition of "ky-universal" module

// ky.d.ts
declare module 'ky-universal' {
  import ky from 'ky';
  export = ky
}

Can we add this definition to fix IDE hits and typescript compiler errors?

@sindresorhus
Copy link
Owner

I think this is a problem with our type definition. I think it should be changed to:

import ky from 'ky';

export = ky;

which is the correct CommonJS syntax.

@ishpartko
Copy link
Author

@sindresorhus

Yes, it can be the solution.

Should I create PR or would you be able to delegate this change to someone?

@sholladay sholladay linked a pull request Apr 20, 2020 that will close this issue
@sholladay sholladay self-assigned this Apr 20, 2020
@sholladay
Copy link
Collaborator

sholladay commented Apr 20, 2020

I have submitted PR #17 to fix that issue @ishpartko. After it is merged and released, your workaround should no longer be necessary.

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

Successfully merging a pull request may close this issue.

3 participants