Support es modules #4232

sigma-andex · 2022-02-11T08:57:09Z

Description of the change

This PR adds es modules support to purescript. It is the rebased pr #3791 from the es modules working group with fixed purity annotations. Solves #3613.

Checklist:

Added a file to CHANGELOG.d for this PR (see CHANGELOG.d/README.md)
Added myself to CONTRIBUTORS.md (if this is my first contribution)
Linked any existing issues or proposals that this pull request should close
Updated or added relevant documentation
Added a test for the contribution (if applicable)

ES modules are already parsed in strict mode.

This reverts commit 7f0c07e.

Node.js loads JavaScript files with a .js extension as CommonJS modules unless they're within a directory with a `"type": "module"` package.json, in which case it loads them as ES modules.

Node.js ignores the package.json file of the output directory otherwise and loads .js files as CommonJS modules.

Node.js allows ES modules to import CommonJS modules by providing the module.exports object as their default export and named exports for statically discoverable properties of the module.exports object. This has an unpleasant consequence for foreign imports: CommonJS exports named `default` are only available as the default property of their default export so a `default :: String` identifier imported from a CommonJS foreign module would actually have type `{ default :: String }`!

The require function and the exports object are not available in ES modules on Node.js.

…ations

This reverts commit 4976eee.

JordanMartinez · 2022-02-24T15:39:24Z

CI is failing because I deleted the es-modules branch for the purescript-lists repo since it doesn't have any FFI. I'll push a commit that reverts it back to 6.0.0 after verifying that the resolutions field doesn't need to also be updated.

JordanMartinez · 2022-02-25T14:44:58Z

For those not watching the working-group-purescript-es/purescript repo, @rhendric opened a PR addressing his concern about the extra IIFEs and his concern about the Pure constructor being separate from the Comment constructor, which hinders optimization passes later, in working-group-purescript-es#15

His comment about the .cjs file extension was not addressed in the above PR.

JordanMartinez · 2022-02-25T18:01:59Z

IIUC, this imperfectly-named variable is the output of the bundler, which is a CommonJS module, and we seem to be sticking to the convention that .js means ES module and .cjs means CommonJS module.

Since we'll be dropping purs bundle, I don't think this issue needs to be dealt with.

rhendric · 2022-02-25T21:08:09Z

Since we'll be dropping purs bundle, I don't think this issue needs to be dealt with.

I was possibly unclear; I was justifying the change, not saying that it's an issue that needs to be resolved. Nets out to the same result as your conclusion though. 😄

rhendric

LGTM (naturally, now that I've gotten my hands into it), but what's going on with this thread about the browser REPL backend?

natefaubion · 2022-02-25T22:15:13Z

@rhendric The browser REPL is effectively broken because it relies on purs bundle. Since we are removing purs bundle in a follow up we need to either decide:

Fix up it's static server to serve individual modules.
Remove the browser REPL altogether.

I'm personally in favor of the second option, because I'm not aware of anyone taking advantage of it, and I find the experience of try-purescript to be so much better for testing things in the browser (try-purescript will need to be updated at some point too). In our discussions, no one expressed interest in maintaining this feature. However a consensus will need to be reached among the core team as a whole. If others on the team are interested in using and maintaining it, that's fine.

rhendric · 2022-02-25T22:19:52Z

I've never used this feature either (and would personally be fine with removing it if due diligence is done), but my understanding is that it's not a REPL in the browser; it's a browser-hosted backend for the console REPL. In a sense it's the inverse of Try PureScript, which is a server-hosted backend with a browser frontend. I think it's potentially uniquely useful for people doing frontend development in PureScript and who want to test some foreign JS integration not supported in Node.js.

natefaubion · 2022-02-25T22:23:41Z

I've never used this feature either (and would personally be fine with removing it if due diligence is done), but my understanding is that it's not a REPL in the browser; it's a browser-hosted backend for the console REPL. In a sense it's the inverse of Try PureScript, which is a server-hosted backend with a browser frontend. I think it's potentially uniquely useful for people doing frontend development in PureScript and who want to test some foreign JS integration not supported in Node.js.

The browser repl just pushes evaluation to the browser. In that sense, it's not different from try purescript, which also evaluates in the browser (it does not evaluate on the backend), except you also have an editor right there as well.

rhendric · 2022-02-25T22:28:14Z

Oh, ignore me then!

JordanMartinez · 2022-02-27T00:02:14Z

v0.14.7 has been released.

JordanMartinez · 2022-02-27T00:37:09Z

I merged the following PRs in the following order to see what merge conflicts might arise. All merge conflicts are minor:

I'll be merging these PRs in the above order once CI passes since all of them are already approved.

rhendric · 2022-02-27T00:39:31Z

#4241 is approved?

JordanMartinez · 2022-02-27T00:52:12Z

Oh, it's not yet. Sorry about that, and thanks for pointing that out.

rhendric · 2022-02-27T02:05:48Z

Is that it for this round of merges? I'll start updating my long PRs if so.

JordanMartinez · 2022-02-27T02:07:40Z

I'm trying to get #4241 in. Then I'd like to merge #4248. After that, yeah.

rhendric · 2022-02-27T02:10:06Z

Sounds good, thanks, and thanks for all the hard work.

JordanMartinez · 2022-02-27T02:21:51Z

Just FYI. https://github.com/purescript/purescript/actions/runs/1904795691 failed to build master because of a lint error. I merged #4240 (which built fine) and then merged #4239 (which didn't rebuild after #4240 was merged). So, maybe that had something to do with it?

Also, if we're spamming this PR and should move future conversations to another issue, could someone open a new issue? I'm responding here because most of the people involved are already subscribed to this thread.

rhendric · 2022-02-27T02:36:36Z

Yeah, the error is from Weeder; what happened is that with both of those PRs merged, we got rid of the last usage of the function that emits warnings during parsing! We can add it to the list of roots in weeder.dhall and/or just merge #4241, which uses it again.

JordanMartinez · 2022-02-27T02:52:21Z

I'm trying to get #4241 in. Then I'd like to merge #4248. After that, yeah.

Since #4241 will need a more in-depth review and may possibly require us to merge a PR dropping purs bundle before it'll build, I'd like to merge #4249 (can I get an approval once CI builds?) to fix CI, and then merge #4248. After that, we'll be done for this round of PRs, I think.

kl0tl added 30 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

Load ES modules with esm in the Node.js REPL and tests

7f0c07e

Escape primes in modules accessors

e006953

Forbid unescaped primes in foreign modules exports

243ec5e

Run tests against patched dependencies

ddbb2ad

Rewrite ES modules in the browser REPL client

e5b1798

Merge branch 'master' into es-modules

47cdee9

Revert "Load ES modules with esm in the Node.js REPL and tests"

c5ffab6

This reverts commit 7f0c07e.

Allow Node.js to load .js files in the output directory as ES modules

4713b2a

Node.js loads JavaScript files with a .js extension as CommonJS modules unless they're within a directory with a `"type": "module"` package.json, in which case it loads them as ES modules.

Import CommonJS foreign modules through an ES module wrapper

7784dd1

Don't let tests nor the REPL compile into a node_modules directory

1587749

Node.js ignores the package.json file of the output directory otherwise and loads .js files as CommonJS modules.

Bundle re-exports

ba9f084

Load bundles as CommonJS modules in tests

65dab5d

Update Node.js version on CI

830bbe2

Disallow CommonJS exports and imports in ES foreign modules

f73c2bf

The require function and the exports object are not available in ES modules on Node.js.

Deprecate CommonJS foreign modules

c56d3f5

Convert CommonJS foreign modules in tests to ES modules

94af221

Don't optimize away dependencies of named ES exports of declarations

15ebf0d

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.

Merge branch 'master' into es-modules+purity-annotations

deec912

rhendric added 3 commits February 25, 2022 17:07

Refactor Pure -> Comment in CoreImp

a1a1eff

Refactor CoreImp modules

9f29b4d

Be less liberal with IIFEs

038108e

thomashoneyman approved these changes Feb 25, 2022

View reviewed changes

rhendric approved these changes Feb 25, 2022

View reviewed changes

JordanMartinez merged commit ff68b93 into purescript:master Feb 27, 2022

rhendric mentioned this pull request Feb 27, 2022

ES modules #3791

Closed

JordanMartinez mentioned this pull request Feb 27, 2022

Temporarily add addWarning to weeder roots to fix CI #4249

Merged

5 tasks

JordanMartinez mentioned this pull request Mar 3, 2022

Use Math.imul(a, b) instead of a * b | 0 #2980

Open

JordanMartinez mentioned this pull request Mar 14, 2022

Add test for 4264 numbers FFI issue #4265

Closed

5 tasks

klntsky mentioned this pull request May 10, 2022

Switch to purescript v0.15 Plutonomicon/cardano-transaction-lib#400

Closed

Support es modules #4232

Support es modules #4232

Uh oh!

Conversation

sigma-andex commented Feb 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JordanMartinez commented Feb 24, 2022

Uh oh!

JordanMartinez commented Feb 25, 2022

Uh oh!

JordanMartinez commented Feb 25, 2022

Uh oh!

rhendric commented Feb 25, 2022

Uh oh!

rhendric left a comment

Choose a reason for hiding this comment

Uh oh!

natefaubion commented Feb 25, 2022

Uh oh!

rhendric commented Feb 25, 2022

Uh oh!

natefaubion commented Feb 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rhendric commented Feb 25, 2022

Uh oh!

JordanMartinez commented Feb 27, 2022

Uh oh!

JordanMartinez commented Feb 27, 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!

JordanMartinez commented Feb 27, 2022

Uh oh!

rhendric commented Feb 27, 2022

Uh oh!

JordanMartinez commented Feb 27, 2022

Uh oh!

rhendric commented Feb 27, 2022

Uh oh!

JordanMartinez commented Feb 27, 2022

Uh oh!

rhendric commented Feb 27, 2022

Uh oh!

JordanMartinez commented Feb 27, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

sigma-andex commented Feb 11, 2022 •

edited

Loading

natefaubion commented Feb 25, 2022 •

edited

Loading

JordanMartinez commented Feb 27, 2022 •

edited

Loading