ES modules #3791

kl0tl · 2020-02-15T19:51:32Z

This PR compiles to ES modules instead of CommonJS.

As summarised in #3613:

Imports are compiled to namespace imports.
Exported identifiers are compiled to named exports.
Primes are always escaped in PureScript modules exports but forbidden in foreign modules.
Both ES and CommonJS foreign modules are supported.

For instance, given this foreign module:

export var message = "Hello world";

the following PureScript module:

module Main where

import Prelude

import Effect (Effect)
import Effect.Console (log)

foreign import message :: String

main :: Effect Unit
main = log message

compiles to:

// Generated by purs version 0.13.6
import * as $foreign from "./foreign.js";
import * as Effect_Console from "../Effect.Console/index.js";
var main = Effect_Console.log($foreign.message);
export {
    message
} from "./foreign.js";
export {
    main
};

and can then be bundled with purs bundle.

kl0tl · 2020-02-15T19:59:47Z

Note that since primes in foreign modules exports are now required to be escaped, this isn’t usable before #3613 (comment) and enough time for the ecosystem to adapt.

triallax · 2020-02-19T11:48:42Z

Isn't the $ after Just unnecesarry right here?

kl0tl · 2020-02-23T12:58:11Z

That $ was indeed redundant!

I’ve rebased the PR on top of #3792 and dropped support for escaped primes in identifiers exported from foreign modules.

hdgarrood · 2020-04-14T00:59:01Z

@kl0tl thanks very much for tackling this, I'm really excited to give it a try! Just to set expectations, I think this is unlikely to get looked at least until v0.14.0 is out. After that, we might want to wait a while before merging this, to give people time to heed the warnings about primes in FFI identifiers, and also because we probably don't want to put breaking releases out too often. We can assess the effect this will have on the ecosystem, though; if it's not too great, that makes it easier to justify releasing ES modules more quickly after v0.14.0. I suspect ES modules won't affect libraries very much, but the migration might be more difficult for some apps, especially if they have complicated bundling setups.

kl0tl · 2020-04-15T19:56:27Z

Regarding the ecosystem: I’ve submitted pull requests to all impacted libraries in the official package set, is there anything more I could do to help with the transition?

hdgarrood · 2020-04-15T20:06:28Z

Thanks! I can’t think of anything really, apart from trying to get lots of people with different setups to try this branch out on their apps and report back on how much effort was required to get them working, so that we can better see how much of an impact this will have on the ecosystem, and see if there’s anything else we can do to make the transition smoother. That should wait until after v0.14.0 is out too though, since people will need to update their apps to v0.14.0 first.

kl0tl · 2020-04-18T00:20:06Z

I successfully bundled one of our projects at work with this branch and webpack@4! The resulting bundle weights 18% less and we’ll scrape a few more percents with webpack@5 and sideEffects: false 🎉

I have to upstream a patch to purs-loader but I hadn’t any change to make to the webpack configuration otherwise, beside what was required for testing webpack@5 and sideEffects: false.

I expect bundling the output directory directly with webpack (without purs-loader), rollup or parcel to work as is. I took a look at rollup-plugin-purs but I don’t think it needs anything to work with ES modules. Also I believe this makes zephyr unnecessary for webpack@5 and rollup (based on https://github.com/kl0tl/purescript-es-modules-benchmark) but I’d be interested to try with a real app.

kl0tl · 2020-04-18T14:48:15Z

I was curious about how zephyr would perform on the same project so I did more tests today and bundling CJS modules optimized by zephyr still beats webpack@5 over ES modules 😅

Here’s a summary of my results with webpack@4:

CJS modules	ES modules	CJS modules + zephyr	ES modules + zephyr
502.16 KB	409.36 KB	379.81 KB	352.58 KB (for some reason the bundle is slightly larger, 352.92 KB, with `sideEffects: false`)

and webpack@5:

CJS modules	ES modules	CJS modules + zephyr	ES modules + zephyr
483.58 KB	394.74 KB (388.17 KB with `sideEffects: false`)	368.26 KB	340.16 KB (339.31 KB with `sideEffects: false`)

I’ll compile zephyr against this branch to see how bundling optimized ES modules compares.

Edit: I’ve updated the table with ES modules optimized by zephyr.

kl0tl · 2020-04-22T21:24:30Z

I picked https://github.com/f-f/purescript-react-basic-todomvc as a simple enough but hopefully not too trivial project to test various bundlers configurations with ES modules.

I have successfully tested (with zephyr when possible) so far:

ES modules are already parsed in strict mode.

kl0tl · 2021-03-21T18:49:58Z

tests/purs/bundle/3551/ModuleWithDeadCode.js

-"use strict";
-
-var fs = require('fs');
+import * as fs from 'fs';


It would be more idiomatic to write import fs from 'fs' or import { readFileSync } from 'fs' here but purs bundle does not support these syntaxes yet.

kl0tl · 2021-03-21T18:50:19Z

tests/purs/bundle/3551/ModuleWithDeadCode.js

-var fs = require('fs');
+import * as fs from 'fs';

 var source = fs.readFileSync(__filename, 'utf-8');


This works because the output of purs bundle is interpreted as a CommonJS module by Node.js but __filename is not available in ES modules, so we’ll have to support CommonJS foreign modules at least until language-javascript can parse import.meta.url.

I wonder how much __filename is used across the ecosystem? I expect it will ruin at least one person's day if we ship this with a version of language-javascript which can't yet parse import.meta.url.

kl0tl · 2021-03-21T18:58:02Z

.travis.yml

 language: node_js
 node_js:
-  - "10"
+  - "14"


We could also specify the version of Node.js with a .nvmrc file (https://docs.travis-ci.com/user/languages/javascript-with-nodejs/#specifying-nodejs-versions-using-nvmrc). This would make it easier to enforce the version to use during development (tests won’t pass anymore on <14.13.0).

That sounds fine as long as we aren't requiring people to use nvm. But I think having the tests fail on <14.13.0 is probably fine too. Why do the tests fail? Does the repl still work on older versions of node?

The tests and the REPL need to be able to import CommonJS foreign modules with ES named imports, but this isn’t possible before v14.13.0:

module Example where example = "Hello world"

> purs repl Example.purs 'bower_components/purescript-*/src/**/*.purs' Compiling Example PSCi, version 0.14.0 Type :? for help > import Example > example

> node Welcome to Node.js v14.2.0. Type ".help" for more information. > import('./.psci_modules/index.js') Promise { <pending> } > (node:28607) UnhandledPromiseRejectionWarning: export { cons, join, showArrayImpl, showCharImpl, showIntImpl, showNumberImpl, showStringImpl } from "./foreign.cjs"; ^^^^ SyntaxError: The requested module './foreign.cjs' does not provide an export named 'cons' at ModuleJob._instantiate (internal/modules/esm/module_job.js:92:21) at async ModuleJob.run (internal/modules/esm/module_job.js:107:20) at async Loader.import (internal/modules/esm/loader.js:179:24)

I did not reconsider the version requirement when introducing ES wrappers for CommonJS foreign modules though, but we could import the default export of the CommonJS foreign module in the ES wrapper and re-export properties of the default export with export declarations:

// foreign.cjs "use strict"; exports.log = function (s) { return function () { console.log(s); }; };

// foreign.js import $foreign from "./foreign.cjs"; var log = $foreign.log; export { log };

This has the advantage of working all the way back to Node.js v12.17.0 (earlier v12 versions need --experimental-modules) which is great news since support for v10 stops at the end of the month!

This approach sounds good to me 👍

How far back is --experimental-modules understood?

Back in 2017, Node.js 8.9.0 shipped experimental support for ECMAScript modules, known for their import and export statements. This support was behind the flag --experimental-modules.

https://nodejs.medium.com/announcing-a-new-experimental-modules-1be8d2d6c2ff

We’re really interested in the implementation of --experimental-modules that was shipped with v12.0.0, this flag is of no use to us in v10.0.0 and earlier.

So if we can expect this approach to work when supplying --experimental-modules with any version of node.js >= v12.0.0, and if earlier versions of node are EOL, this sounds fine to me. I think we might want to check the version of node we are using in the repl and supply that flag where necessary. Perhaps we should test on node v12 as well as v14 for now as well?

The REPL and the tests now throw on unsupported Node.js versions (<12.0.0) and provide the --experimental-module flag for versions >=12.0.0 && <12.7.0.

Perhaps we should test on node v12 as well as v14 for now as well?

We don’t rely on the behaviour introduced in v14.13.0 anymore, so testing on v12.0.0 and the latest v12 might be enough?

src/Language/PureScript/Bundle.hs

…ations

This reverts commit 4976eee.

kl0tl · 2021-07-03T13:58:40Z

I merged master and implemented the strategy described in this comment to support Node.js >=12.0.0!

JordanMartinez · 2021-07-03T14:43:37Z

Nice!

Just FYI. CI failures are caused by hlint finding a few things.

squidCatx · 2021-12-13T03:19:51Z

what is the reason to stop merge this

JordanMartinez · 2021-12-13T20:23:31Z

@cocoddy It's not yet done. I don't recall everything, but one open question is what to do about purs bundle.

thomashoneyman · 2022-01-17T19:59:55Z

Node.js has made great progress on the interoperability of CommonJS and ES modules though: ES namespace imports are able to import CommonJS exports since v14.13.0, which is great news for us! Most of my work this weekend has been about ensuring CommonJS and ES foreign modules can coexist natively on Node.js

I think we should support only ES modules in FFI for simplicity's sake.

This means that CommonJS modules would not be accepted in FFI files, and we'd attempt no automatic conversions or processing to have ES modules and CommonJS work together. As far as the compiler is concerned it's all ES modules in and ES modules out.

However, the world doesn't work this way: plenty of browser and Node projects currently rely on CommonJS and we need to be sure that they continue to work even if the compiler only accepts ES modules. Still, I think we're in the clear:

We can omit the browser part of this question because browsers do not support CommonJS; you will have to use a bundler if you need CommonJS for the browser, and bundlers support both module types.
I believe we can omit the Node part of this question, since Node supports both module types from Node 14 onward and we have evidence from the node-challenge that mixing module types works fine.

To use a CommonJS module in your FFI, you would need to take one of two routes. The first is where the CommonJS is using named exports:

// commonjs uses named exports
// mymodule.js
exports.foo = ...
exports.bar = ...

// es modules FFI file can't do named imports directly; you'd have to import the
// full module then pick off what you need
import mymodule from "mymodule.js"

export const foo = mymodule.foo;
export const bar = mymodule.bar;

The second is where the CommonJS is using a default export:

// commonjs uses default export
// mymodule.js
module.exports = ...

// es modules
import mymodule from "mymodule.js"

export default mymodule;

Either way, it's possible to write your FFI in such a way that you only write an ES module, and if you need to pull in CommonJS dependencies then you rely on either a bundler in the case of the browser (which is required anyway), or rely on Node's module resolutions. Either way we can get away with only supporting ES modules.

Further reading:
https://redfin.engineering/node-modules-at-war-why-commonjs-and-es-modules-cant-get-along-9617135eeca1

tiye · 2022-01-18T01:53:54Z

better using

import mymodule from "./mymodule.js"

since "mymodule.js" is treated as a package(then an extra package.json will be involved), rather than a file.

rhendric · 2022-02-27T01:28:07Z

Closed in favor of #4232.

kl0tl mentioned this pull request Feb 15, 2020

ES modules #3613

Closed

kl0tl force-pushed the es-modules branch 3 times, most recently from a5ea971 to f42d247 Compare February 16, 2020 15:14

kl0tl force-pushed the es-modules branch from f42d247 to a0d067d Compare February 23, 2020 12:11

gasi mentioned this pull request Feb 28, 2020

Styled tags collegevine/purescript-elmish-html#6

Merged

kl0tl mentioned this pull request Mar 7, 2020

Deprecate primes in identifiers exported from foreign modules #3792

Merged

kl0tl force-pushed the es-modules branch from a0d067d to 815a3bc Compare April 6, 2020 20:14

kl0tl marked this pull request as ready for review April 6, 2020 20:31

kl0tl force-pushed the es-modules branch from 815a3bc to 3fdbd6d Compare April 20, 2020 18:29

kl0tl mentioned this pull request Apr 22, 2020

Support ES modules ethul/purs-loader#145

Merged

kl0tl added 9 commits April 22, 2020 23:25

Add ES imports/exports to CoreImp AST

6c481c8

Print ES imports/exports

52a692b

Codegen ES imports for PureScript modules

cd40596

Codegen ES imports for foreign modules

607dd3d

Codegen ES exports

19878e3

Extract both CJS and ES exports from foreign modules

4889861

Remove the redundant "use strict;" pragma from modules header

c03399f

ES modules are already parsed in strict mode.

Don’t emit empty statements for empty exports lists

71c2de3

Bundle ES modules

7d4ef3d

kl0tl force-pushed the es-modules branch from 3fdbd6d to b00e31a Compare April 22, 2020 21:38

kl0tl commented Mar 21, 2021

View reviewed changes

hdgarrood reviewed Apr 8, 2021

View reviewed changes

src/Language/PureScript/Bundle.hs Show resolved Hide resolved

milesfrain mentioned this pull request May 31, 2021

address the CommonJS module system purescript-contrib/purescript-book#356

Merged

i-am-the-slime reviewed Jun 17, 2021

View reviewed changes

src/Language/PureScript/Bundle.hs Outdated Show resolved Hide resolved

kl0tl added 8 commits July 3, 2021 15:09

fixup! Import CommonJS foreign modules through an ES module wrapper

8f19b7e

fixup! Don't optimize away dependencies of named ES exports of declar…

7727c98

…ations

Revert "Disallow CommonJS exports named default"

bd45602

This reverts commit 4976eee.

Add tests for foreign CommonJS exports named default

ca94c4b

Extend support to Node.js v12.0.0 with --experimental-modules

ec4dce5

Filter out Node.js experimental ES modules loader warning

0d6f928

Update bundler error messages

8e1caf8

Merge branch 'master' into next

7ef3aad

kl0tl mentioned this pull request Jul 3, 2021

Purs bundle drops var but keeps const and let #4039

Closed

Fix HLint warnings

d5437e0

klarkc mentioned this pull request Aug 19, 2021

Implement ref example klarkc/pure-vue#2

Closed

thomashoneyman mentioned this pull request Jan 16, 2022

Proposal: Remove the PureScript bundler #4226

Closed

This was referenced Jan 18, 2022

Proposal: Load foreign code with ES Module Shims and import maps purescript/trypurescript#263

Closed

Proposal: Introduce esbuild for Try PureScript purescript/trypurescript#264

Closed

thomashoneyman mentioned this pull request Jan 29, 2022

Proposal: Only support ES modules in PureScript 0.15 (disallow CommonJS) #4230

Closed

sigma-andex mentioned this pull request Feb 11, 2022

Support es modules #4232

Merged

5 tasks

rhendric closed this Feb 27, 2022

ES modules #3791

ES modules #3791

Uh oh!

Conversation

kl0tl commented Feb 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kl0tl commented Feb 15, 2020

Uh oh!

triallax commented Feb 19, 2020

Uh oh!

kl0tl commented Feb 23, 2020

Uh oh!

hdgarrood commented Apr 14, 2020

Uh oh!

kl0tl commented Apr 15, 2020

Uh oh!

hdgarrood commented Apr 15, 2020

Uh oh!

kl0tl commented Apr 18, 2020

Uh oh!

kl0tl commented Apr 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kl0tl commented Apr 22, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kl0tl Apr 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kl0tl commented Jul 3, 2021

Uh oh!

JordanMartinez commented Jul 3, 2021

Uh oh!

squidCatx commented Dec 13, 2021

Uh oh!

JordanMartinez commented Dec 13, 2021

Uh oh!

thomashoneyman commented Jan 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tiye commented Jan 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rhendric commented Feb 27, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants

kl0tl commented Feb 15, 2020 •

edited

Loading

kl0tl commented Apr 18, 2020 •

edited

Loading

kl0tl Apr 10, 2021 •

edited

Loading

thomashoneyman commented Jan 17, 2022 •

edited

Loading

tiye commented Jan 18, 2022 •

edited

Loading