Open-Attestation / open-attestation

Meta framework for providing digital provenance and integrity to documents.
https://openattestation.com
Apache License 2.0
54 stars 18 forks source link

BUG: ESM imports not working #195

Closed Neketek closed 3 years ago

Neketek commented 3 years ago

Version 6.0.0 package.json is missing the "main" key which prevents the library from being imported. It was in package.json before, I assume it's being missing is just a typo.

Nothing else to add, pretty straightforward to fix.

Nebulis commented 3 years ago

It was not a typo, I wanted to upgrade to a full ESM library, but I currently face many issues. Please stick with 5 in the meantime

Neketek commented 3 years ago

It was not a typo, I wanted to upgrade to a full ESM library, but I currently face many issues. Please stick with 5 in the meantime

@Nebulis Ye, thank you. I was just going to post about this, but you did it first. Is there any way to make v6 work in typescript? Because as I understood ESM is not really supported, at least now according to the state of this issue.

Nebulis commented 3 years ago

Your link goes to the current issue.

I faced issues with node, ts-node and jest. I'm investigating because I want to share ea full report on the attempt for our team.

I will probably fix the v6 beginning of next week. (Mostly a full revert)

Neketek commented 3 years ago

Your link goes to the current issue.

I faced issues with node, ts-node and jest. I'm investigating because I want to share ea full report on the attempt for our team.

I will probably fix the v6 beginning of next week. (Mostly a full revert)

Ye, sorry, I've updated the link. Thanks for the info, it's very useful for planning a course of action.

Nebulis commented 3 years ago

Your link is interesting. I didn't have any issues with importing OA from a different TS library. And some people said it's working for them too (for instance https://github.com/microsoft/TypeScript/issues/33079#issuecomment-872431024)

For the record, what issue do you currently face?

Neketek commented 3 years ago

@Nebulis So, the issue is simple, it's unable to resolve the module in jest and in webpack. According to my linters and autocompletion engine everything is alright. And it's right because the module is in place. I've tried to switch target and module to es2020, but It didn't help.

image

P.S. I thought that the issue is the "main" field because the first thing I've done after I understood that the problem might be inside the package is checking the previous version which worked just fine.

Nebulis commented 3 years ago

Ok you have the same issue that I found. It's because jest doesn't support ESM (https://github.com/facebook/jest/issues/9771, https://github.com/facebook/jest/issues/9430).

I made it work without typescript, I need to try with TS.

I haven't tested webpack, but I thought it was supported. Now I need to try ... :)

Neketek commented 3 years ago

image @Nebulis And here is what I get when I'm trying to build using webpack. Webpack itself doesn't specify anything special to be done in order to work with ESM modules, so I think it might be related to how you reference modules inside of the package.

Nebulis commented 3 years ago

It actually looks like the second error I faced. Unlike node resolve mechanism, ESM does not by default resolve dir by loading index.js, or resolve the extension.

For instance with node, you need https://nodejs.org/api/esm.html#esm_customizing_esm_specifier_resolution_algorithm (an experimental flag) and TS doesn't provide a way to generate the correct full path. TS answers is: developers must provide the correct expected full path, ie in your code you should have something like: import "./3.0/digest/index.js". Yes according to TS dev, you mus import JS files in your TS code :)

Maybe there is a flag in webpack for this too. Thanks fr the sharing

Neketek commented 3 years ago

@Nebulis I've made it work with webpack, and probably found the reason why it didn't work initially. image As far as I understand, according to this comment you really need to change how you reference your code inside the package for it to be ESM compliant. So apparently, the new standard requires imports to use fully specified names. Maybe, if you change this it will start working with jest and ts without additional tweaking.

Nebulis commented 3 years ago

You are correct for the webpack config (I did the same) and the full path. Having correct full path help to solve most of the issues indeed.

john-dot-oa commented 3 years ago

:tada: This issue has been resolved in version 6.0.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket: