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

Support ESM. #55

Open
524c opened this issue Oct 3, 2023 · 6 comments
Open

Support ESM. #55

524c opened this issue Oct 3, 2023 · 6 comments

Comments

@524c
Copy link

524c commented Oct 3, 2023

Hi @icebob, how are you?

I tried to use moleculer-io in an ESM project and I couldn't, so I decided to generate a port that is working well. The tests passed, although it displays a warning.

In addition to making some adjustments to the code, I used tsup to generate, in addition to exporting, an ESM version, and also generate a CommonJS version.

I added the build script that will create the dist folder.

My fork is here: https://github.com/524c/moleculer-io

If you could take a look and tell me what you think, I'd appreciate it.

@524c
Copy link
Author

524c commented Oct 3, 2023

This was my first pass through the code and there are certainly better ways to do this, but it was something that resolved my issue quickly. So I would like to know what you think and if we can have support from ESM :)

@icebob
Copy link
Member

icebob commented Oct 8, 2023

could you show a diff from the changes and how it will be deployed to NPM?

@524c
Copy link
Author

524c commented Oct 8, 2023

Hi, here is a link with the diff.

I didn't update to NPM. For now, I'm using this fork and an internal "npm" called Verdaccio.

If you think the change is good enough and accept contributions I would consider making a PR, after any necessary adjustments.

master...524c:moleculer-io:master

@524c
Copy link
Author

524c commented Oct 8, 2023

Currently all updated tests run successfully in ESM, but report a warning.

--------------|---------|----------|---------|---------|---------------------------------------------------------
File          | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                                       
--------------|---------|----------|---------|---------|---------------------------------------------------------
All files     |      90 |     72.8 |   96.55 |   90.64 |                                                         
 constants.js |     100 |      100 |     100 |     100 |                                                         
 errors.js    |     100 |       75 |     100 |     100 | 21                                                      
 index.js     |   89.53 |    72.72 |   96.29 |   90.18 | 178-179,217,330-331,338,419,435-436,479-480,498,521-545 
--------------|---------|----------|---------|---------|---------------------------------------------------------

Test Suites: 2 passed, 2 total
Tests:       20 passed, 20 total
Snapshots:   0 total
Time:        1.371 s
Ran all test suites.
Jest did not exit one second after the test run has completed.

'This usually means that there are asynchronous operations that weren't stopped in your tests. Consider running Jest with `--detectOpenHandles` to troubleshoot this issue.

@icebob
Copy link
Member

icebob commented Oct 9, 2023

as I see, all import codes changed to ESM and tsup generates CJS version, right?
Can we change the logic to keep everything in CJS and generate ESM with tsup?

@524c
Copy link
Author

524c commented Oct 9, 2023

@icebob

I have two branches now:

  1. original code and than used tsup to build cjs + esm: master...524c:moleculer-io:master
  2. code converted to esm before and than used tsup to build cjs + esm: master...524c:moleculer-io:esm

I did the following in the item 1:
In the master branch, apart from an update to a test that returned a warning about a deprecated method, I kept the original code and configured tsup to export the cjs and esm versions in dist.

When I import the ESM version that was generated from CJS (1), for some reason I still have a problem with the import:

[0] [Runner] Dynamic require of "socket.io" is not supported Error: Dynamic require of "socket.io" is not supported
[0]     at file:///broker/node_modules/moleculer-io/dist/index.mjs:7:9
[0]     at src/index.js (file:///broker/node_modules/moleculer-io/dist/index.mjs:56:26)
[0]     at __require2 (file:///broker/node_modules/moleculer-io/dist/index.mjs:10:50)
[0]     at file:///broker/node_modules/moleculer-io/dist/index.mjs:515:16
[0]     at ModuleJob.run (node:internal/modules/esm/module_job:194:25)
[0] yarn run service-api exited with code 1

With option 2, 100% esm, I have no problem importing the cjs and esm versions.

In my fork I have the new esm branch.

As soon as I can I will continue looking at this.

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

No branches or pull requests

2 participants