Moving CJS to ES6 Modules

[Cmdv]

I've just been experimenting with generating ES6 modules, but before I went any further (add tests etc) wanted some opinions 😄

Think rather than a 1 to 1 conversion it would be great for users to opt in with say a flag -es6?
If this is a good idea could someone point me to an example of something using a flag.

Also if you look below at the output of the JS I ended up using:

import name from '../file/name.js'

instead of:

import * as name from '../file/name.js'

When I tested this * as format with Webpack 4 it just came up with lots of crazy warnings:

WARNING in ./output/Data.Foldable/index.js 182:33-51
"export 'Nothing' (imported as 'Data_Maybe') was not found in '../Data.Maybe/index.js'
 @ ./output/Effect.Aff/index.js
 @ ./output/Main/index.js

The second issue was that foreign exports needed to be an object literal with key values and I wasn't able to use ES6 version of just:
export { a, b , c} because I'm not importing like import {a, b, c} from '../foreign.js'
but I got around this by keeping the current functionality and adding default keyword to the export.

So far the output is as such:

"use strict";
import Button from "../Button/index.js";
import Control_Bind from "../Control.Bind/index.js";
import Data_Unit from "../Data.Unit/index.js";
import Effect from "../Effect/index.js";
import Effect_Aff from "../Effect.Aff/index.js";
import Halogen_Aff from "../Halogen.Aff/index.js";
import Halogen_Aff_Util from "../Halogen.Aff.Util/index.js";
import Halogen_VDom_Driver from "../Halogen.VDom.Driver/index.js";
import Prelude from "../Prelude/index.js";
var main = Halogen_Aff_Util.runHalogenAff(Control_Bind.bind(Effect_Aff.bindAff)(Halogen_Aff_Util.awaitBody)(function (v) {
    return Halogen_VDom_Driver.runUI(Button.myButton)(Data_Unit.unit)(v);
}));
export default{
    main: main
};

Then foreign:

// Generated by purs version 0.12.0
"use strict";
import $foreign from "./foreign.js";
export default {
    abs: $foreign.abs,
    acos: $foreign.acos,
    asin: $foreign.asin,
    ...
};

I tested this against Halogen Basic Demo with Webpack 4 with > webpack --mode production
and the output file was still fairly large at 651 KiB so I'm not sure it's doing all of the DCE / Tree shacking properly.

related to #1206 ¯\(ツ)/¯

[Cmdv]

Ok going to change things a little as the DCE isn't working with how I have things currently:

Normal export will be:

var cube =  function (x) {
  return x * x * x;
}
export { cube };

Foreign exports will need to go be a little more explicit:

export const abs = $foreign.abs

[spicydonuts]

TypeScript has an option for choosing the output target (es2015, es5, etc). If it's just changing the export style es-modules might be more clear.

[paf31]

I'm glad you started looking at ES6 imports and exports, but the real problem is DCE and bundling, as you've found out. There has been a lot of discussion about this in the past, and we decided that someone should go prototype a backend for ES6 first (so it's great that you have!). This can exist as a fork and be useful regardless of whether we can solve the DCE problem in the short term.

Fixing DCE will involve changing purs bundle, and (the hard bit), language-javascript to support ES6 syntax.

[chexxor]

Looks like the language-javascript has a PR (erikd/language-javascript#64) which adds partial ES6 support. I wonder if it's ok to use a branch of that project for this project's needs.

[michaelficarra]

@chexxor There's also the possibility that I proposed long ago of implementing just an import/export extractor for JS modules. That's all we care about, right? This would allow us to be much lazier about parsing. We don't care to actually parse the full module as long as we can accurately skip the parts we are uninterested in.

[spicydonuts]

Which turns out to be simpler and safer with es modules than cjs 🙂

[Cmdv]

I'm now pretty close but have a couple of niggly errors:

// foreign.js
exports["insertCell'"] = function () {}

// index.js
import $foreign from "./foreign.js";

export const insertCell' = $foreign["insertCell'"];

doesn't like insertCell' because ' is an invalid identifier according to ECMAScript 6.

and the other is a hanging ; which I'm sure could be sorted out with correct output for this:

match (ExportStandard _ []) = return $ emit ""

I've tried various solutions with the way you can export + import but it still seems that I'm not getting DCE to work. I'm hopeful that maybe if I can get around the above it might fix it but I'm not too certain that is going to be the case.

[spicydonuts]

Should the ffi file use modules as well? Either way, I've seen ' renamed to $prime elsewhere in the generated code.

// foreign.js
export const insertCell$prime = function () {};

// index.js
import { insertCell$prime } from "./foreign.js";

export const insertCell$prime = insertCell$prime;

[michaelficarra]

I'd be fine with just disallowing foreign imports of invalid JS identifiers when you're backed by a JS implementation.

[Cmdv]

@spicydonuts that would be a good idea, been giving that a go.

@michaelficarra just thinking of backwards compatibility really, because using prime is quite a common thing in PS / Haskell.

I've managed to change the export but just searching for it's use like:
Data_Lens_Prism["prism'"] as I would need to change these too.

[Cmdv]

I'm going to park this for the moment as not finding the time to finish it. Don't think I was too far away but will re attempt in the future if no one else has already done it 😄

[utkarshkukreti]

Hi @Cmdv,

Thanks for the work on this! I just tried out your branch (with a tiny patch to fix a bug, mentioned at the end) on the Halogen basic example. After running webpack -p on the main module, the bundle output was 341K, compared to pulp browserify --optimise which produced a 354K file. I believe the size should go down even further if the foreign files start using ES6 modules (I don't think webpack is able to remove any dead code in foreign files). Could you tell me what things need to be done before this is considered ready? I might be able to help!

Tiny patch:

--- a/src/Language/PureScript/CodeGen/JS.hs
+++ b/src/Language/PureScript/CodeGen/JS.hs
@@ -166,7 +166,7 @@ moduleToJs (Module _ coms mn _ imps exps foreigns decls) foreign_ =
   -- a PureScript identifier. If the name is not valid in JavaScript (symbol based, reserved name) an
   -- indexer is returned.
   accessor :: Ident -> AST -> AST
-  accessor (Ident prop) = accessorString $ mkString prop
+  accessor (Ident prop) = accessorString $ mkString $ properToJs prop
   accessor (GenIdent _ _) = internalError "GenIdent in accessor"
   accessor UnusedIdent = internalError "UnusedIdent in accessor"

(Your code exports a function named const as $$const but the code to access that property outputs ["const"] without this patch.)

[Cmdv]

@utkarshkukreti thanks I was literally thinking about starting this off again today.

Think I went towards converting all keywords to using $$ as I still had a discrepancy between how things were being Imported + Exported.

I can't recall if I did more work on this locally and broke things even further haha. I'll have a look and respond as this would be a cool feature 😄

Right so just been having a look, re you're last comment:

(Your code exports a function named const as $$const but the code to access that property outputs ["const"] without this patch.)

I've just checked and it does seem to output ["$$const"] though technically that's not required as it could quite easily do FOO.$$const()

The other part that I have an issue with is before it used to do 2 different types of exports but now it only does one and I couldn't work out where this trailing ; came from:

export {
    main
};
;

just feels like with this working it should be able to be a lot more performant than this, do you feel it's more down to how the FFI's are not using ES6? I might try it out see if it does make a difference 😄

[Cmdv]

right so having experimented with converting an FFI to using ES6 export instead of CJS and it fails. This is probably done by the parser too so will need the ability to handle ES6 too, yet still being backwards compatible otherwise nothing will work unless all FFI code is converted to ES6!! that for sure doesn't sound fun.

FutureToken {tokenSpan = TokenPn 31760 1140 1, tokenLiteral = "export", tokenComment = [WhiteSpace (TokenPn 31758 1138 27) "\n\n"]}

[utkarshkukreti]

Yeah, looks like this is gonna require some more work. The language-javascript package recently got support for ES6 import/export statements. The compiler needs to be modified to support that.

I was able to get webpack to work transparently with both ES6 and CommonJS modules:

➜ cat Main.js
import * as First from './First';
import * as Second from './Second';

console.log(First.first, Second.second);
➜ cat First.js
module.exports = { first: 1 };
➜ cat Second.js
export const second = 2;
➜ webpack Main.js
...
➜ node dist/main.js
1 2

[gabejohnson]

@utkarshkukreti looks like that merge was reverted in favor of implementing the features individually erikd/language-javascript#71

[utkarshkukreti]

Ah, you're right. No support for import/export yet. 😞

[Cmdv]

that's cool though I might do it as the work has already been done just needs an individual PR 😄

[Cmdv]

@utkarshkukreti not sure if you or anyone else would be keen but now have 2 PR's on language-javascript for import + export but need a bit of help as getting a bit stuck!