Rewriting CommonJS foreign modules of core/contrib/web libraries

[i-am-the-slime]

Originally written by @thomashoneyman

Rewriting CommonJS foreign modules is straightforward but the exact format into which we should rewrite them depends on what we do with purs bundle.

If we were to drop our bundler we could rewrite exports.ƒ = function () { to export const ƒ = function () { (as @i-am-the-slime started to do in purescript/purescript-prelude#264) or export function ƒ() {.

But if we are to keep our bundler we could rewrite this to export function ƒ() { -- only once purescript/purescript#4044 is merged -- and other declarations should be exported as vars (unless we extend the bundler to support lets, consts and other declarations).

As I said earlier, I think it's really unlikely anything happens to the bundler in the near future, so the second option is the one that's probably the way forward.

[i-am-the-slime]

VSCode can automatically convert modules:

Which greatly simplifies this process.

[thomashoneyman]

That's great to know! So long as the conversion it generates fits with the requirements we have here (we aren't going to drop the bundler at least for 0.15).

[JordanMartinez]

This blog post mentions how to support Node since it expects the FFI file's names to be .mjs. Working around that requires adding a type: "module" part to the package.json file: https://blog.jim-nielsen.com/2020/switching-from-cjs-to-esm/

[JordanMartinez]

There's also other tools that can update FFI files from the terminal:

• https://github.com/getify/moduloze
• https://github.com/wessberg/cjstoesm

[thomashoneyman]

This blog post mentions how to support Node since it expects the FFI file's names to be .mjs. Working around that requires adding a type: "module" part to the package.json file: https://blog.jim-nielsen.com/2020/switching-from-cjs-to-esm/

Node users therefore would have to either (1) write FFI files with the .mjs extension or (2) ensure they have a package.json file and add a type: "module" field, is that correct?

As far as the compiler is concerned, it would need to accept either .js or .mjs files for the FFI then.

[hdgarrood]

Whenever JS FFI code is being run by Node, I think it should only ever be picked up from the output directory, not from the src directory. It sounded to me like we are planning to write a package.json with "type":"module" in the output directory - if so, I don’t think .mjs for FFI will be necessary. I think .mjs is a (hopefully temporary) Node-specific aberration and it’d be preferable not to build in knowledge of it to the compiler if we can avoid it.

[thomashoneyman]

Yes, I should have followed up here — having looked back at the original PR I saw that we do put that package.json in the output and therefore no need for .mjs

[i-am-the-slime]

Only problems I've run into with the VSCode approach is that I had to be careful for it not to insert trailing commas, because language-javascript can't parse those.
Would probably be nice to have in the long run but not a showstopper for now at all.

[hdgarrood]

If we are going to drop purs bundle - and it seems like that is the direction things are going - that doesn’t matter, right? We can drop the language-javascript dependency completely, so it doesn’t matter whether or not it can parse things.

[JordanMartinez]

But dropping language-javascript means we won't have a way to verify that FFI actually exists for some foreign import functionName, which is a good sanity check. So, if that dependency was dropped, we'd need a way to support the check.

[hdgarrood]

I think a bundler should be able to do that, and if you’re not bundling in dev I think it should result in an error being thrown immediately (but we should probably check that). In my opinion, the language-javascript dependency is enough of a burden that I’d be happy to lose that check; a big part of what makes dropping purs bundle appealing is being able to drop that dependency.

[JordanMartinez]

I think a bundler should be able to do that, and if you’re not bundling in dev I think it should result in an error being thrown immediately (but we should probably check that).

If that's true, it would save us quite the headache!

[sigma-andex]

I just did a small test by changing helloFromPSESMviaFFI to helloFromPSESMviaFFI2 in the generated index.js

and it seems that at least esbuild is able to catch that: