moleculerjs / moleculer-io

Socket.io API GateWay service for Moleculer framework
MIT License
79 stars 30 forks source link

Support ESM. #55

Open 524c opened 9 months ago

524c commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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

524c commented 9 months ago

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.

https://github.com/moleculerjs/moleculer-io/compare/master...524c:moleculer-io:master

524c commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

@icebob

I have two branches now: 1) original code and than used tsup to build cjs + esm: https://github.com/moleculerjs/moleculer-io/compare/master...524c:moleculer-io:master 2) code converted to esm before and than used tsup to build cjs + esm: https://github.com/moleculerjs/moleculer-io/compare/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.