Conversation
|
This also fixes testdouble/testdouble.js#513 |
|
Hmm... Windows not working. Luckily, I have a Window machine now, so checking... |
searls
left a comment
There was a problem hiding this comment.
This is a big change! Looks like you modernized the style and APIs in use a bit with the refactor -- thank you!
test/esm-lib/quibble-esm.test.mjs
Outdated
|
|
||
| console.log('3') | ||
| const result = await import('../esm-fixtures/a-module-with-function.mjs') | ||
| console.log('4') |
There was a problem hiding this comment.
Can probably clear these console logs when everything is working
|
@searls sorry about the mess. The work on the Windows side (and a good night's sleep 😊 made me realize the code's design is problematic. I will probably spend today on some level of refactoring and get it to work correctly. So I'm guessing the above comments won't really have any meaning once I finish refactoring and making it work on both platforms, but I promise I will go over them once its done. BTW, there are two main problems I see on the windows side:
Luckily I have windows machine with both WSL and Windows, so I can navigate back and forth between them easily. |
ee09d20 to
7b0919e
Compare
|
Major refactoring after it failed (spectacularly!) in Windows. The code is MUCH improved, and Windows compatibility should be much better going forward, because now all ESM code deals with URLs and not file paths (see more information about this in the PR description) Ready for re-review! |
| module: await importFunctionsModule.importOriginalModule(fullImportPath) | ||
| // The name of this property _should_ be `moduleUrl`, but it is used in `testdouble` as `modulePath` | ||
| // and so can't be changed without breaking `testdouble`. So I add another field with the correct name | ||
| // and once testdouble is updated, I can remove the `modulePath` field. |
There was a problem hiding this comment.
Sounds good. Let's update quibble and then peg TD to it so you can update it there too
|
Released as quibble@0.7.0 and testdouble@3.18.0 |
|
It looks like this PR drops node 14 support intentionally. I was trying to use |
|
Node 14 is EOL since this April, and I can tell you from watching @giltayar work that getting ESM to function across 14, 16, 18, and 20 has required an incredible amount of patching and branching. Please expect this library to aggressively drop support for EOL'd versions so that we can keep the codebase simple and maintainable |
Fixes #95
Node.js 20 moved the loaders off the main thread, to their own thread. Usually this is not a problem for loaders, but
quibblehas an API that communicates with the loader itself. In versions prior to 20, this communication was done using global variables, but since the loaders are in a separate worker/thread, then this is not possible (workers/threads do not share global variables)The only option for communication is to use messages between the main thread and the worker threads. Luckily, the loader team gave loaders the ability to do just that.
This PR enables the loader API to communicate with the loader using messages, if the loaders are off thread, and reverts to the previous method (global variables) if the loaders are in the main thread.
I took the liberty of removing crud related to ESM loaders in past versions. Also, the Github action now checks the current versions of Node, which are 16, 18, and 20.
I have also cleaned up the code and made Windows support easier by making all ESM code use URLs instead of file paths, because 1) that is the native language ESM speaks, 2) It is compatible with Windows, and 3) now that we can have HTTP (and other) urls as import specifiers, then we are not allowed to assume that the files are on disk!