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

Typescript / Angular constructor errors #53

Closed
N-Andronopoulos opened this issue Apr 4, 2020 · 24 comments
Closed

Typescript / Angular constructor errors #53

N-Andronopoulos opened this issue Apr 4, 2020 · 24 comments
Labels
help wanted Extra attention is needed

Comments

@N-Andronopoulos
Copy link

N-Andronopoulos commented Apr 4, 2020

Describe the bug
Import error/confusion.

Litepicker 1.0.34
Typescript 3.8
Angular 9

To Reproduce
If you import like this: import { Litepicker } from 'litepicker'; in typescript (angular)
and use it as const picker = new Litepicker(..). I get:

ERROR in ..../src/app/main-nav/service-overview/service-overview.component.ts:41:24 - error TS2351: This expression is not constructable. Type 'typeof import("...../node_modules/litepicker/dist/js/index")' has no construct signatures.

If I import it as import * as Litepicker from 'litepicker';

Then compiler errors with: TS2351: This expression is not constructable.   Type 'typeof import("/..../node_modules/litepicker/dist/js/index")' has no construct signatures.

(I also tried import Litepicker from 'litepicker'; which is not valid)

I think you have the types mixed up or something.

edit
I think you need a contructor defined in methods.ts
Litepicker.prototype = [...] something like that.

Expected behavior
To run without errors or warnings.

@wakirin
Copy link
Owner

wakirin commented Apr 6, 2020

Maybe comments will useful here: https://wakirin.github.io/Litepicker/#anchor-Comments
Feel free to publish your solution if you know how to fix it.
Thanks.

@wakirin wakirin added the help wanted Extra attention is needed label Apr 11, 2020
@Plysepter
Copy link

Plysepter commented Apr 16, 2020

I haven't looked too deep into it just yet but my current work around is as follows:

import Litepicker from 'litepicker';
import { Litepicker as LPicker } from 'litepicker';

...

private litePicker: LPicker;

...

this.litePicker = new (Litepicker as any)({...})

I am unable to post the entire component, however these are the key lines of code that got it to work for myself. Hopefully this helps unblock you until a cleaner solution can be provided

@Plysepter
Copy link

I should update that this works for builds but for some reason errors out when running unit tests reporting that litepicker.default is not a constructor. I am looking into possible solutions and will update accordingly

@Plysepter
Copy link

So I have some updates on why it behaves as it currently does:

The TS2351: This expression is not constructable. Type 'typeof import("/..../node_modules/litepicker/dist/js/index")' has no construct signatures. error is due to the way the interface is being interprited by TS. methods.ts is using an interface that is missing a construct signature. More insight can be found here.

As for the import statement I am still having issues in Typescript. My work around above works great for my browser build but when I go to unit test the component I receive TypeError: litepicker_1.default is not a constructor. I am presuming there is a difference in how it is transpiled that is causing the error here.

A possible solution would be to move all the public functions from methods.ts into the base class in litepicker.ts and change index.ts to look like either:

import { Litepicker } from './litepicker';

export default Litepicker;

or just set litepicker.ts to be the main file itself.

This would allow TS to pull the typing from the class directly instead of from a separate interface and make it so that import statements should look the same for both JS & TS implementations (import Litepicker from 'Litepicker').

I am happy to create the PR to do so, however I know that there is a trade off of readability. I think it was a smart decision to separate the public methods into their own file like it currently is since TS doesn't support partial classes at this time. That being said I think it might be worth merging the files for the time being.

@wakirin I would like to ask for your input on whether or not you feel this would be the right direction for the project. If you agree, I am happy to start on a PR, otherwise I am happy to work towards other possible solutions.

@wakirin
Copy link
Owner

wakirin commented Apr 18, 2020

Thanks for research.
I will think about it.

@wakirin
Copy link
Owner

wakirin commented Apr 18, 2020

Well, I think found a working solution.
Check please the branch https://github.com/wakirin/Litepicker/tree/fix-import
Usage:

import Litepicker from 'litepicker';
...

new Litepicker({
    element: document.getElementById('datepicker'),
})

@Plysepter
Copy link

Looks great I tested the changes against my repo and it works in production but still errored out in tests. I found out this is because I do not have the esModuleInterop setting set to true in my tests tsconfig.json.

After some digging I have discovered that removing (commenting out) the libraryExport setting in webpack.common.js's output settings resolves this issue. With it commented out I was able to run the library in my production code, my tests, the libraries tests and the libraries documentation site successfully.

So my suggestion would be that we remove the library export setting. If that is not viable we should look into updating the docs to note that the library may require the tsconfig setting when using typescript to work.

@wakirin
Copy link
Owner

wakirin commented Apr 20, 2020

I see that without setting libraryExport, the JS version doesn't work.
I added the esModuleInterop setting to tsconfig.json and it is works the same way (Angular and JS works).
Have you tried adding this setting to library instead of your tsconfig.json ?

@Plysepter
Copy link

So after some further testing I have come to the conclusion that esModuleInterop must be enabled in the ts config of the project importing the library if they are experiencing errors (I believe it is when transpiling within a node environment but my stack is a bit too complex to confirm atm).

I am not an expert with webpack but from my investigation it appears that we have to choose between either this tsconfig setting or making the JS users use the library like Litepicker.Litepicker when loading from a script tag. Which would be a breaking change.

I think what is in the fix-import branch as it currently is resolves the issue. We may want to update the documentation noting that if you are experiencing errors with Typescript to try adding esModuleInterop to your tsconfig.json but I don't think there is much to do beyond that for the time being.

Maybe when the library is reaching a point where v2 is being planned, it could be considered whether switching to a Litepicker.Litepicker solution for ES5 JS is worth while. Maybe if something like #1 is implemented similar to import { Datepicker, Timepicker } from 'litepicker' it would make sense but for now I think the issue can be marked as resolved once the new version is live.

@wakirin
Copy link
Owner

wakirin commented Apr 20, 2020

Ok, I'll merge the branch fix-import.
Thanks for your help and research.

@wakirin
Copy link
Owner

wakirin commented Apr 20, 2020

v1.2.0 has been published.

@Plysepter
Copy link

Plysepter commented Apr 20, 2020

No worries, happy to help! And thank you for being so responsive and receptive!

@nightire
Copy link

Confirmed v1.2.0 works well even without "esModuleInterop": true.

@wakirin wakirin closed this as completed Apr 30, 2020
@CaffeineDreams
Copy link

Is there any alternative to the "esModuleInterop": true solution? I added it to our tsconfig and it broke some of our other dependencies, most notably, and somewhat ironically, moment! It's technically fixable, but less than ideal to have to modify every single file where we import moment (this is a large project).

I'm using 1.5.5

@wakirin
Copy link
Owner

wakirin commented Jun 23, 2020

@CaffeineDreams
"esModuleInterop": true is optional, have you tried without it ?

@CaffeineDreams
Copy link

Yeah without that its behaving as described in the first post in this thread. 2 main differences in our setup though - we're on TS 3.5.3 and Angular 8.2, but I'm pretty sure angular has nothing to do with this.

I'm following basically the same pattern as OP:

import { Litepicker } from 'litepicker';
...

private datePicker: Litepicker;

public ngOnInit() {
   ...
   this.datePicker = new Litepicker({...});
   ...
}

It builds fine but we get a runtime error:
TypeError: litepicker_1.Litepicker is not a constructor

Thanks for responding :)

@wakirin
Copy link
Owner

wakirin commented Jun 23, 2020

@CaffeineDreams
Could you create a repository in which this problem will be reproduced ?
I don't ask to publish your entire project, you can create some basic app with dependencies of your project.
Thanks.

@CaffeineDreams
Copy link

Yeah I can do, will be a few hours until I get a chance though.

@CaffeineDreams
Copy link

Here's a stackblitz showing the behaviour:

https://stackblitz.com/edit/angular-ivy-hmlygj

You'll see the error in the console on page load

@wakirin
Copy link
Owner

wakirin commented Jun 24, 2020

@CaffeineDreams
Import Litepicker as stated in the docs. (without brackets)

import Litepicker from 'litepicker';

@CaffeineDreams
Copy link

hmmm.... I'm sorry but while that works on the stackblitz example, when I try that in my project I get:
Uncaught TypeError: litepicker_1.default is not a constructor

@CaffeineDreams
Copy link

I've been having another stab at this today. It would appear that the global Litepicker function is available and working correctly at runtime, as I can call var foo = new Litepicker(...) from the browser console just fine. It's only when I import as per my previous comment that I get the error.

@CaffeineDreams
Copy link

Ok, I got to the bottom of it and it's, as I suspected, entirely a problem at our end. We use Angular with a custom webpack config, and our chunking configuration was missing the Litepicker import. I added a line to import litepicker in our vendor.ts file and it's working now.

Thank-you for your help

@Alex0298
Copy link

Alex0298 commented Dec 15, 2021

Hello I am trying to use the library in vue js but it generates a dataset error, the truth is that it is the first time that I am going to use it, it looks very good but I probably need to do some configuration or something, thank you very much.

Vue warn]: Error in created hook: "TypeError: Cannot read properties of null (reading 'dataset')"

found in

---> at src/views/ViewCapacitacion/index.vue
at node_modules/potencie-libraries/src/layouts/PotencieMark.vue
at src/App.vue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

6 participants