Parse ES2015 (aka ES6) `export` Declarations (partial support) #79

gasi · 2018-11-18T06:22:58Z

First of all, thanks for creating and maintaining this wonderful library! 😄

Motivation

I am a passionate PureScript user and would like to see the project adopt ES2015 (aka ES6) for better bundling and interop with the rest of the JavaScript ecosystem. See:

As a first step, I’d like to support parsing export declarations (statements) that could be used to write hand-written PureScript FFI files with ES2015 modules instead of CommonJS, e.g. https://github.com/purescript/purescript-prelude/blob/dc1069417b04896a1fb8b90175e67f91e1d9051a/src/Record/Unsafe.js

Prior Art

@Cmdv started this PR #78 but it had a few compile errors, so I decided to start with a new branch based onnew-ast:

~/workspace/language-javascript/src/Language/JavaScript/Process/Minify.hs:63:48: error:
    • No instance for (MinifyJS (Maybe JSAnnot))
        arising from a use of ‘fixSpace’
    • In the second argument of ‘JSExport’, namely ‘(fixSpace df)’
      In the expression: JSExport a (fixSpace df) (fixEmpty x1) s
      In an equation for ‘fixStmt’:
          fixStmt a s (JSExport _ df x1 _)
            = JSExport a (fixSpace df) (fixEmpty x1) s
   |
63 | fixStmt a s (JSExport _ df x1 _) = JSExport a (fixSpace df) (fixEmpty x1) s -- unsure what I'm doing here
   |                                                ^^^^^^^^^^^

# ...

Implementation Notes

The implementation follows the Exports specification in section 15.2.3 of the ECMAScript 2015 specification: http://www.ecma-international.org/ecma-262/6.0/ECMA-262.pdf#page=304

Current Support

ExportDeclaration
- export * FromClause ;
- export ExportClause FromClause ;
- export ExportClause ;
- export VariableStatement
- export Declaration
- export default HoistableDeclaration[Default]
- export default ClassDeclaration[Default]
- export default [lookahead ∉ { function, class }] AssignmentExpression[In] ;
Examples
- export {}
- export const a = 1;
- export { a };
- export { X as Y };

Feedback

Since I don’t have a background in compilers (minus a college class long-time ago), I’d love to get some feedback on my current approach before I continue.

Questions

Should JSExport be part of JSStatement or should we create JSDeclaration (or similar)? In the spec ExportDeclaration is part of:

Module :
  ModuleBodyopt
  
ModuleBody :
  ModuleItemList

ModuleItemList :
  ModuleItem
  ModuleItemList ModuleItem

ModuleItem :
  ImportDeclaration
  ExportDeclaration
  StatementListItem

and statements are defined as:

Statement :
  BlockStatement
  VariableStatement
  EmptyStatement
  ExpressionStatement
  IfStatement
  BreakableStatement
  ContinueStatement
  BreakStatement
  ReturnStatement
  WithStatement
  LabelledStatement
  ThrowStatement
  TryStatement
  DebuggerStatement

but I’m not sure what the ideal relationship between the AST and the grammar should be.

Would you be open to support stack besides just cabal? I couldn’t get the project to build using cabal-install:

> cabal --version && cabal update && cabal clean && cabal configure && cabal build
cabal-install version 2.2.0.0
compiled using version 2.2.0.1 of the Cabal library
Downloading the latest package list from hackage.haskell.org
To revert to previous state run:
    cabal update --index-state='2018-11-17T17:40:28Z'
cleaning...
Resolving dependencies...
Warning: solver failed to find a solution:
Could not resolve dependencies:
[__0] trying: language-javascript-0.6.0.11 (user goal)
[__1] unknown package: utf8-string (dependency of language-javascript)
[__1] fail (backjumping, conflict set: language-javascript, utf8-string)
After searching the rest of the dependency tree exhaustively, these were the
goals I've had most trouble fulfilling: language-javascript, utf8-string
Trying configure anyway.
Configuring language-javascript-0.6.0.11...
cabal: Encountered missing dependencies:
blaze-builder >=0.2, utf8-string >=0.3.7 && <2

Does as make sense as binary operator? I modelled it after in which can be used both for 'foo' in {foo: 2} (binary operator) as well as for (var x in xs) {...} (not really a binary operator?).
Minor: Could we use the term ES2015 instead of ES6 (and ES2016 instead of ES7)? The former seems to be the official name and other projects such as Babel have adopted as well: https://babeljs.io/docs/en/learn#ecmascript-2015-features

This fixes the messaging as LHS is actual and RHS expected test result.

gasi

Annotated for review, @erikd and @Cmdv.

gasi · 2018-11-18T06:25:58Z

src/Language/JavaScript/Parser/AST.hs

    | JSWhile !JSAnnot !JSAnnot !JSExpression !JSAnnot !JSStatement -- ^while,lb,expr,rb,stmt
    | JSWith !JSAnnot !JSAnnot !JSExpression !JSAnnot !JSStatement !JSSemi -- ^with,lb,expr,rb,stmt list
+    | JSExport !JSAnnot !JSExportBody !JSSemi -- ^export, body, autosemi


Is introducing JSExportBody to support the following variants the right abstraction?

-- ExportDeclaration : See 15.2.3 -- [ ] export * FromClause ; -- [ ] export ExportClause FromClause ; -- [x] export ExportClause ; -- [x] export VariableStatement -- [ ] export Declaration -- [ ] export default HoistableDeclaration[Default] -- [ ] export default ClassDeclaration[Default] -- [ ] export default [lookahead ∉ { function, class }] AssignmentExpression[In] ;

Q: What makes for the right AST abstraction?

Not sure. You decide :)

Decided to revisit this a bit and opened #80. Let me know what you think.

src/Language/JavaScript/Parser/AST.hs

gasi · 2018-11-18T06:31:09Z

src/Language/JavaScript/Parser/Grammar7.y

@@ -887,6 +897,7 @@ StatementNoEmpty : StatementBlock      { $1 {- 'StatementNoEmpty1' -} }
                 | ThrowStatement      { $1 {- 'StatementNoEmpty13' -} }
                 | TryStatement        { $1 {- 'StatementNoEmpty14' -} }
                 | DebuggerStatement   { $1 {- 'StatementNoEmpty15' -} }
+                 | ExportDeclaration   { $1 {- 'StatementNoEmpty16' -} }


This doesn’t align with the ECMAScript 2015 spec but it seems the approach that #64 has taken:
https://github.com/erikd/language-javascript/pull/64/files#diff-843804451967a2b03f95fbef01a330d1R905

My first attempt created a separate JSDeclaration data type 9330863 but then I reverted to align with the existing code.

My maintenance of this has focused more on parsing real world JS rather than following the spec. This is fine.

Thanks for clarifying. If not following the spec, how do you decide whether a program should pass or fail parsing? I thought most modern JS engines follow the spec but as I said, I don’t have a lot of background in this space.

test/Test/Language/Javascript/RoundTrip.hs

erikd · 2018-11-18T09:06:05Z

You happy with this as it is now?

Cmdv · 2018-11-18T20:57:47Z

@gasi ❤️, been meaning to complete this just not had the time to doing it really happy you are taking it on. Let me know if I can be of help as this has given me motivation!

erikd · 2018-11-18T20:58:40Z

Don't blame me, all the credit goes to @gasi :) .

Cmdv · 2018-11-18T21:09:49Z

@ totally the wrong person hahaha will amend!

gasi · 2018-11-19T00:38:23Z

Thanks for the review, @erikd, and for the shout out, @Cmdv. I revisited this slightly in #80 based Shift AST API, i.e. exposing Module and parseModule as top-level constructs.

If you’re happy with #80, I will close this PR.

erikd · 2018-11-21T21:08:30Z

Closed in favor of #80

gasi added 8 commits November 17, 2018 20:35

Fix typos

28aaad3

Flip test condition

d4f2b6d

This fixes the messaging as LHS is actual and RHS expected test result.

gitignore: Ignore .stack-work

6312561

Fix indentation

e96a436

Parse empty export declaration

9330863

Merge JSDeclaration with JSStatement

f082ec6

Parse export lists and variable statements

eb2b798

🎨 Improve line length

4fe6f05

gasi commented Nov 18, 2018

View reviewed changes

Update copyright

580990e

gasi mentioned this pull request Nov 18, 2018

Better documentation for dead code elimination purescript/purescript#2545

Closed

gasi mentioned this pull request Nov 19, 2018

ES2015: Modules: export declarations (partial) #80

Closed

erikd closed this Nov 21, 2018

gasi deleted the new-ast-es2015-export-declaration branch June 2, 2019 18:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Parse ES2015 (aka ES6) `export` Declarations (partial support) #79

Parse ES2015 (aka ES6) `export` Declarations (partial support) #79

Uh oh!

gasi commented Nov 18, 2018

Uh oh!

gasi left a comment

Uh oh!

gasi Nov 18, 2018

Uh oh!

erikd Nov 18, 2018

Uh oh!

gasi Nov 19, 2018

Uh oh!

Uh oh!

gasi Nov 18, 2018

Uh oh!

erikd Nov 18, 2018

Uh oh!

gasi Nov 19, 2018

Uh oh!

Uh oh!

erikd commented Nov 18, 2018

Uh oh!

Cmdv commented Nov 18, 2018 •

edited

Loading

Uh oh!

erikd commented Nov 18, 2018

Uh oh!

Cmdv commented Nov 18, 2018

Uh oh!

gasi commented Nov 19, 2018

Uh oh!

erikd commented Nov 21, 2018

Uh oh!

Uh oh!

Parse ES2015 (aka ES6) export Declarations (partial support) #79

Parse ES2015 (aka ES6) export Declarations (partial support) #79

Uh oh!

Conversation

gasi commented Nov 18, 2018

Motivation

Prior Art

Implementation Notes

Feedback

Questions

Uh oh!

gasi left a comment

Choose a reason for hiding this comment

Uh oh!

gasi Nov 18, 2018

Choose a reason for hiding this comment

Uh oh!

erikd Nov 18, 2018

Choose a reason for hiding this comment

Uh oh!

gasi Nov 19, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gasi Nov 18, 2018

Choose a reason for hiding this comment

Uh oh!

erikd Nov 18, 2018

Choose a reason for hiding this comment

Uh oh!

gasi Nov 19, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

erikd commented Nov 18, 2018

Uh oh!

Cmdv commented Nov 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

erikd commented Nov 18, 2018

Uh oh!

Cmdv commented Nov 18, 2018

Uh oh!

gasi commented Nov 19, 2018

Uh oh!

erikd commented Nov 21, 2018

Uh oh!

Uh oh!

Parse ES2015 (aka ES6) `export` Declarations (partial support) #79

Parse ES2015 (aka ES6) `export` Declarations (partial support) #79

Cmdv commented Nov 18, 2018 •

edited

Loading