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

error TS2497: Module ''jimp'' resolves to a non-module entity and cannot be imported using this construct. #581

Closed
atas opened this issue Aug 29, 2018 · 22 comments · Fixed by #582 or #660
Labels
bug there is a bug in the way jimp behaves

Comments

@atas
Copy link

atas commented Aug 29, 2018

Hi,

I cannot import jimp with typescript, getting error. Details are below.

Expected Behavior

There shouldn't be a compilation-time error.

Current Behavior

There is a compilation-time error.
error TS2497: Module ''jimp'' resolves to a non-module entity and cannot be imported using this construct. import * as jimp from 'jimp';

Also, import jimp from 'jimp'; doesn't work because it imports the jimp into jimp_1 and becomes not usable...

Failure Information (for bugs)

Steps to Reproduce

Import jimp in a typescript file with import * as jimp from 'jimp';

  • Jimp Version: ^0.3.9
  • Operating System: MacOS
  • Node version: v8.11.3
  • TypeScript version: 3.0.1

Failure Logs

error TS2497: Module ''jimp'' resolves to a non-module entity and cannot be imported using this construct. import * as jimp from 'jimp';

hipstersmoothie added a commit that referenced this issue Aug 30, 2018
# What's Changing and Why

fixes typescript build

## What else might be affected

closes #581

## Tasks

- [x] Add tests
- [x] Update Documentation
- [x] Update `jimp.d.ts`
- [x] Add [SemVer](https://semver.org/) Label
@hipstersmoothie
Copy link
Collaborator

released in v0.3.10

@hipstersmoothie
Copy link
Collaborator

This actaully doesn't seem to work fully.

Now when i try to import in a non ts file it is exported under default

Do you know of any way to get this to work @atas ?

declare module 'jimp' {
  export = Jimp.Jimp;
  export default Jimp.Jimp;
}

@hipstersmoothie
Copy link
Collaborator

I gotta go back to because the latest release broke vscode's autocompletion.

declare module 'jimp' {
  export = Jimp.Jimp;
}

I've found you can make <0.3.10 work with the following

import Jimp = require('jimp');

@hipstersmoothie
Copy link
Collaborator

I've found if you do either of the following in your tsconfig.json <0.3.10 tsc build works and autocompletions in both environments work

  "compilerOptions": {
    "esModuleInterop": true,
    "allowSyntheticDefaultImports": true
  },

So with that I am going to revert back to the code in 0.3.9

@atas
Copy link
Author

atas commented Aug 30, 2018

Hi,

I've updated to the release v0.3.10 defined above but now I have bigger problems.

Issues

(1) If I set "esModuleInterop": true, half of the project gets broken, most of other libraries complain with A namespace-style import cannot be called or constructed, and will cause a failure at runtime. Consider using a default import or import require here instead.

So I am not sure if adding "esModuleInterop": true, in TS conf is the right solution, however import jimp = require('jimp') did work so if we revert we can go ahead with that.

(2) I tried import jimp from 'jimp'; with the version 0.3.10 (and not import * as jimp), intellisense works and expect jimp variable to be defined but the jimp variable gets undefined during runtime so I get "read()" does not exist on undefined error for jimp.read() for instance.

(3) I tried import * as jimp2 from 'jimp'; on this version again, TS intellisense expects jimp2.default to be the jimp variable but when I debugged the code I see jimp2 itself is the variable, so intellisense is mislead, hence either runtime or compilation error happens.

(4) I tried import jimp3 = require('jimp'); with this version again, same behaviour as (3).

Resolution Path

  • Would it be possible that before we make a release we do a PR first and we together review that is working, both runtime and compile time first?

  • Whatever works can we put that in the README file as a guide to how to use TypeScript with that and we stick with that standard?

If you open the PR and add me as well I will contribute too @hipstersmoothie and will confirm what works compile and runtime. Intellisense may work or project may compile but there might be runtime errors still, so better to check it all the way just to be sure.

Thank you,

Ata

@hipstersmoothie
Copy link
Collaborator

hipstersmoothie commented Aug 30, 2018 via email

@hipstersmoothie hipstersmoothie added the bug there is a bug in the way jimp behaves label Aug 31, 2018
@hipstersmoothie
Copy link
Collaborator

@atas ping

@atas
Copy link
Author

atas commented Sep 1, 2018

Hey @hipstersmoothie , I will have a look on Monday and Tuesday, on holiday right now :)

I actually got this working previously in a PR here #427 but that was closed in favour of another one which I just tested recently but I will go over again and create another PR soon.

If you welcome the idea maybe we should tidy up the type definition file because it is done with a class rather than interfaces.

For instance, entry point is a class type and by using its static methods we return an instance of the class... That makes it difficult to extend as they are of the same type.

You can define a class in typescript in ambient context and use it like a type definition but better to use interfaces, more flexible. Would you be open to a PR making that big of a change?

@hipstersmoothie
Copy link
Collaborator

I definitely open to a rewrite of the types file. I recently broke the reop into a mono repo structure where everything is now a plugin. It would be really cool if each plugin/type had it's own typedef file that extended the main jimp type file!

@favna
Copy link
Contributor

favna commented Nov 9, 2018

I'd like to revive this issue for Jimp v0.5.2 with tsc v3.1.6. Currently I'm using import Jimp from 'jimp'; however the Jimp in runtime then turns out as undefinedthus I cannot access simple functions likeJimp.read()`.

@hipstersmoothie
Copy link
Collaborator

Have you tried importing in other ways? @favna

@favna
Copy link
Contributor

favna commented Nov 11, 2018

Actually yes, I have since figured out that it works with import Jimp = require('jimp');. It ended up being documented on typescriptlang

Extract:

Both CommonJS and AMD generally have the concept of an exports object which contains all exports from a module.

They also support replacing the exports object with a custom single object. Default exports are meant to act as a replacement for this behavior; however, the two are incompatible. TypeScript supports export = to model the traditional CommonJS and AMD workflow.

The export = syntax specifies a single object that is exported from the module. This can be a class, interface, namespace, function, or enum.

When exporting a module using export =, TypeScript-specific import module = require("module") must be used to import the module

IMO it feels off to be using require in TypeScript but if we would like to use import * as Jimp from 'jimp' or such a more ES6 Import/Export structure the definitions file would have to be rewritten a lot I'm afraid. I tried messing around myself a lot but I couldn't get it that's for sure :\

@hipstersmoothie
Copy link
Collaborator

Yeah i can write typescript code but have had no luck getting .d.ts files to work right all the time. The require feels off to me too. I've tried messing with the .d.ts file too with no avail. If anyone tries to fix this these requirements still stand:

Requirements:

  • intellisense works in CJS project without having to require('jimp').default
  • intellisense works in TSC project
  • TSC can build the project without errors (and preferably without any special flags)

@pndewit
Copy link

pndewit commented Nov 16, 2018

Not really an expert, but the following seems to fix most of my issues.

Add the following to the top of the jimp.d.ts file: import * as jimp from 'jimp';
Remove this from the jimp.d.ts file:declare module 'jimp' { export = Jimp.Jimp; }

Let me know if this helps in any way.

@hipstersmoothie
Copy link
Collaborator

hipstersmoothie commented Nov 17, 2018

@favna @atas Can you verify that the change suggested by @pndewit works?

If it does feel free to put a pr together and I'll get it merged!

@favna
Copy link
Contributor

favna commented Nov 18, 2018

@hipstersmoothie it "works" but that's honestly stretching the definition. That it to say, the TS compiler throws this (replace "read" with whatever) on all functions and properties from Jimp Property 'read' does not exist on type 'typeof import("e:/UserFiles/DevProjects/ribbon/node_modules/jimp/jimp")'. [2339].

image

and when excluding the import * as jimp from 'jimp'; from the jimp.d.ts as well the TS compiler says [ts] File 'e:/UserFiles/DevProjects/ribbon/node_modules/jimp/jimp.d.ts' is not a module. [2306] when importing into the file where the lib should be used. Again it "works" but not without compiler issues and errors in, at least, VSCode and IntelliJ IDEA.


That all said, I've done some more digging into declaration files myself and I've come a pretty clean solution of my own too.

  • In jimp.d.ts at the bottom remove declare module 'jimp' { export = Jimp.Jimp; }
  • In jimp.d.ts at the bottom add declare var exportedJimp = Jimp.Jimp; and export = exportedJimp;
  • In the files using the lib add import * as Jimp from 'jimp'; (here the Jimp after as is replaceable by whatever the end-user wants, it's like const X = whatever)
  • Use lib as normal, for example Jimp.read(...)

If you give the green light I'll set up a PR for this solution.

Edit: I have a branch with the change ready for PR here

@hipstersmoothie
Copy link
Collaborator

hipstersmoothie commented Nov 19, 2018

Sweet! Make a PR and I will get to it in the next few days! @favna

@pndewit
Copy link

pndewit commented Nov 19, 2018

@favna Nice, your solution is indeed working better. I am getting a TS error though:
TS1039: Initializers are not allowed in ambient contexts.
screen shot 2018-11-19 at 08 02 36

@favna
Copy link
Contributor

favna commented Nov 19, 2018

@pndewit ai... I didn't see that one. It does work when using it and I've compiled code successfully too. Maybe we should just add // @ts-ignore and be done with it?

@pndewit
Copy link

pndewit commented Nov 19, 2018

I'm cool with just ignoring it. I am reading a little bit about the issue we're trying to solve here and it seems like our solution is a little hacky though: https://stackoverflow.com/questions/39415661/what-does-resolves-to-a-non-module-entity-and-cannot-be-imported-using-this

@favna
Copy link
Contributor

favna commented Nov 19, 2018

I'll add the ignore and create the PR but I'll keep this issue open (will rewrite the commit message to remove the "resolves" part). I think the ideal case would be to rewrite the declarations file, specifically by extracting the class Jimp from the namespace Jimp and basically restructuring it entirely.

@favna
Copy link
Contributor

favna commented Nov 28, 2018

@kritollm please read issue comments before commenting yourself. It is known that your method works, as this is the documented method from typescriptlang (scroll to the section about require). The critique however is that using "require" (a CommonJS method of importing) is ugly when the rest of your code uses proper TypeScript supported ES6 Import/Export. Furthermore using this CommonJS scheme breaks the Babel loader used by jsdoc2md to parse JSDoc in TypeScript files.

Suffice it to say it is a workaround for a problem that should not exist.

This issue however gets entirely addressed by my PR #660 which will allow both import Jimp = require('jimp') and import * as Jimp from 'jimp' (latter under the condition that the compiler option allowSyntheticDefaultImports is set to true).

hipstersmoothie pushed a commit that referenced this issue Nov 28, 2018
* Improve TypeScript declarations file

Addresses but does not fully close #581

Proper way to import is now `import * as X from 'jimp'` wherein X can be
whatever the end-user wants, in most cases `Jimp`.

* Truly resolve declarations file

I have losely based this on knowledge gained from analyzing other
declarations files, primarily @types/better-sqlite3. Using this
formatting for declaration file allows for `import * as Jimp from
'jimp'` importing which is a proper way when using TypeScript as it
primarily recommends ES6 imports. It will require setting the
`allowSyntheticDefaultImports` compiler option to `true`, however this
is a very small issue as many packages on npm (including aforementioned
better-sqlite3) require this.

- Like previous commit this ensures we no longer need to use `import
Jimp = require('jimp'). As has been agreed upon, using `require` when
working with ES6 modules is just wrong in code style.

- Unlike previous commit no more ts-ignore, yay!

- It is maintainable. New types or interface come at the bottom and any
new or modifications to functions and constants go in the topmost
interface. We'll never have to touch the declared const or the export.
hipstersmoothie added a commit that referenced this issue Nov 28, 2018
* Improve TypeScript declarations file

Addresses but does not fully close #581

Proper way to import is now `import * as X from 'jimp'` wherein X can be
whatever the end-user wants, in most cases `Jimp`.

* Truly resolve declarations file

I have losely based this on knowledge gained from analyzing other
declarations files, primarily @types/better-sqlite3. Using this
formatting for declaration file allows for `import * as Jimp from
'jimp'` importing which is a proper way when using TypeScript as it
primarily recommends ES6 imports. It will require setting the
`allowSyntheticDefaultImports` compiler option to `true`, however this
is a very small issue as many packages on npm (including aforementioned
better-sqlite3) require this.

- Like previous commit this ensures we no longer need to use `import
Jimp = require('jimp'). As has been agreed upon, using `require` when
working with ES6 modules is just wrong in code style.

- Unlike previous commit no more ts-ignore, yay!

- It is maintainable. New types or interface come at the bottom and any
new or modifications to functions and constants go in the topmost
interface. We'll never have to touch the declared const or the export.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug there is a bug in the way jimp behaves
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants