Support 3rd party modules using both ESM and CommonJS#2946
Support 3rd party modules using both ESM and CommonJS#2946buxxi wants to merge 3 commits intoMagicMirrorOrg:developfrom
Conversation
|
Thanks for the brainstorming first of all. But reading "the module alias for node_helper isn't working, the module needs to specify the file with import NodeHelper from "../../js/node_helper.js"; which is annoying" is a little understatement :-) Its quite impossible to get modules to use a new syntax without leaving a lot of old ones behind (and users possibly angry) Using a more modern syntax is probably the way to go but maybe we need to make small steps (like your async work which looks like some stuff I try out too in my branches). |
|
Could move most of the async stuff out to a separate PR to make the original intent smaller and possibly ignore my second point for now and only make this about the node_helpers. This wasn't intended to break any of the existing modules (CommonJS modules should still load fine) and doing ESM modules will always be a bit different (no And I agree that the relative path is not an ok solution, just what I had to do for my test to actually work (as this is only a draft). Will have to figure out why the tests segmentation faults too :) |
2dacbac to
b620175
Compare
… next test (#2952) When trying to debug why the tests broke for #2946 I found that the tests does not wait for the app to start and close. So if the startup isn't blocking that would fail. So I added a callback for `close()` too and converted them to promises for the `startApplication()` and `stopApplication()` and updated all the e2e tests to await both. Will try to refactor all these callbacks to promises in a later PR.
|
Since I failed to solve the prerequisite PR #2962 I will close this for now. |
The problem we had before with node-fetch migrating to an ESM got me thinking about what it would require to make 3rd party modules ESM. So I made this to test having the node_helper being either CommonJS or ESM.
If naming the file
node_helper.mjsinstead ofnode_helper.jsit should load it as ESM instead.This is just a draft, there are a few things left:
import NodeHelper from "../../js/node_helper.js";which is annoying<script type="module".../>, probably a bit harder to have the dual support of .mjs and .js without any configuration...