Skip to content

Partial ES 6 & 7. #64

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jun 16, 2018
Merged

Partial ES 6 & 7. #64

merged 7 commits into from
Jun 16, 2018

Conversation

diasbruno
Copy link
Contributor

@diasbruno diasbruno commented Jan 6, 2018

This implements features for es 6 & 7.

  • const and let
  • import
  • export
  • arrows
  • default arguments
  • object matching (destructuring, single name bind)
  • rest + spread
  • classes + extends
  • template strings

Issue #59.

@diasbruno
Copy link
Contributor Author

I'll leave this PR open so everyone can help me. :)

@diasbruno diasbruno changed the title WIP: Es6 & 7. WIP: ES 6 & 7. Jan 6, 2018
testStmt "if (1) {}" `shouldBe` "Right (JSAstStatement (JSIf (JSDecimal '1') (JSStatementBlock [])))"
-- fix: fix ambiguity with block and object literal on if block position
-- testStmt "if (1) {x}" `shouldBe` "Right (JSAstStatement (JSIf (JSDecimal '1') (JSStatementBlock [JSIdentifier 'x'])))"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erikd I need some help with this. I still can't figure it out a way to disambiguiate block and object on a if statement. The minified version would print something like:

if(a){{x}}...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inner {} is the object.

Copy link
Contributor Author

@diasbruno diasbruno Jan 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably this happens because it will try ObjectLiteral first (?).

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with minor sensible changes to the tests as needed.

@pedrofurla
Copy link
Contributor

I am in need of arrow functions support, is there anything I can do to move help move this forward?

@diasbruno
Copy link
Contributor Author

@pedrofurla Sure. If you can make progress with this PR...please. I didn't have much time to finish it. I wrote language-js just for what I needed, but I hope this one can be finished as well.

@pedrofurla
Copy link
Contributor

To be honest I am happy with its current state, but I will give it a go tomorrow and see what I can do. I make not promises. :)

@dcastro
Copy link

dcastro commented May 10, 2018

@diasbruno, thank you so much for looking into this!

I tried pulling this branch to build my project, and I'm getting this error:

src/Language/JavaScript/Process/Minify.hs:(133,5)-(160,100): Non-exhaustive patterns in function fix

Looks like the MinifyJS instance for JSExpression is missing a case for JSArrowExpression and JSSpreadExpression?

dcastro added a commit to dcastro/twenty48 that referenced this pull request May 11, 2018
hjsmin / language-javascript don't yet support ES 6/7, so had to remove all "arrow functions"
Related: erikd/language-javascript#64
@diasbruno
Copy link
Contributor Author

@dcastro I've implemented just the 'parser' part and few things just to compile/run tests.

@michaelficarra
Copy link

@diasbruno Do you think it would be better to split off the unimplemented features into a separate effort and get this PR into a merge-able state? Partial support of new features is definitely better than no support.

@erikd
Copy link
Owner

erikd commented Jun 11, 2018

I'd be find with partial support.

@diasbruno
Copy link
Contributor Author

diasbruno commented Jun 12, 2018

That would be great. I was begin to worry about not getting time to finish this.
@erikd Please, let me know how can we get this 'first part' done.

Thank you all for the help.

@diasbruno diasbruno changed the title WIP: ES 6 & 7. Partial ES 6 & 7. Jun 12, 2018
@erikd
Copy link
Owner

erikd commented Jun 12, 2018

I would like to do a really through review of this before merging. Hopefully get around to this on the weekend.

@erikd erikd merged commit 7ea8501 into erikd:new-ast Jun 16, 2018
@erikd
Copy link
Owner

erikd commented Jun 17, 2018

Going to have to revert this from the new-ast branch for now. Its not complete. There are incomplete pattern matches in the pretty printer and minifier.

Will look at this myself today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants