decentralized-identity / veramo

A JavaScript Framework for Verifiable Data
https://veramo.io
Apache License 2.0
414 stars 130 forks source link

WIP: Feat/esbuild #1328

Closed cre8 closed 5 months ago

cre8 commented 5 months ago

What issue is this PR fixing

fixes #1321

Compared to this PR this is a more cleaner approach to show the current status.

Current problem with ESBUILD:

file:///C:/Users/mollik/Projects/veramo/packages/core-types/build/types/ICredentialStatusManager.js:1 import { DIDDocument, DIDResolutionOptions, DIDResolutionResult } from "did-resolver"; ^^^^^^^^^^^ SyntaxError: The requested module 'did-resolver' does not provide an export named 'DIDDocument' at ModuleJob._instantiate (node:internal/modules/esm/module_job:124:21) at async ModuleJob.run (node:internal/modules/esm/module_job:190:5)

Node.js v18.17.0


Comparing the `ICredentialStatusManager.js` compiled with esbuild and tsc shows some differences:
tsc:

export {}; //# sourceMappingURL=ICredentialStatusManager.js.map

esbuild:

import { DIDDocument, DIDResolutionOptions, DIDResolutionResult } from "did-resolver"; export { DIDDocument, DIDResolutionOptions, DIDResolutionResult }; //# sourceMappingURL=ICredentialStatusManager.js.map



Even when removing it from the `ICredentialStatusManager.ts` file, it will throw similar errors why types or so that are generated locally.

At this point I am not sure if this can be solved by a parameter that has to be passed to esbuild or changed in tsconfig or if we need to update the commands for the following tasks.

## Quality
Check all that apply:
* [X] I want these changes to be integrated
* [ ] I successfully ran `pnpm i`, `pnpm build`, `pnpm test`, `pnpm test:browser` locally.
* [ ] I allow my PR to be updated by the reviewers (to speed up the review process).
* [ ] I added unit tests.
* [ ] I added integration tests.
* [ ] I did not add automated tests because _________, and I am aware that a PR without tests will likely get rejected.

## Details
If applicable, add screen captures, error messages or stack traces to help explain your problem.
mirceanis commented 5 months ago

It would be much easier to test the migration of a single package to esbuild and to see if it actually improves things. I suggested the @veramo/utils package as it is relatively isolated, but still uses tricky dependencies. It's the dependencies that have caused the most issues for folks.

cre8 commented 5 months ago

Despite the fact that esbuild is fast, I am getting a lot of problems when I don't only want it for compilation, but also being interested in the types etc. I also tried the approach with compiling it with tcs for esm and in parallel with tsc for commonjs. Problem here is to pack both in one npm package. When you add "type": "module" to the package.json, it is treating all files with an '.js' ending as esm, even when they are in the folder for cjs. So according to the error you need to update the file endings for all files that are commonjs from '.js' to '.cjs'. This is not supported by tsc out of the box. I found some projects, but they seem not to be so stable anymore.

Maybe with a bundler like rollup or babel this could work, but I don't have the time right now to dig deeper.

martines3000 commented 5 months ago

For building both ESM and CommonJS I would recommend Tsup. I'm not sure if it resolves the issues here, but it is a great tool that solves a lot of issues regarding building/bundling for multiple environments. It is also fast as it uses ESBuild under the hood.