Skip to content
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

Switch to Acorn #200

Closed
28 of 29 tasks
nzakas opened this issue Sep 15, 2015 · 19 comments
Closed
28 of 29 tasks

Switch to Acorn #200

nzakas opened this issue Sep 15, 2015 · 19 comments

Comments

@nzakas
Copy link
Member

nzakas commented Sep 15, 2015

tl;dr I'm stopping work on bugs to convert Espree over to be powered by Acorn.

There's a lot of time and energy required to maintain a parser. It was never the intent of Espree to need to exist and since Esprima came back to life, the intent was to wait for Esprima to provide the hooks Espree needed to make the changes we'd need to continue functioning in the same way. That's taken a lot longer than we thought (still not on the roadmap), and so Espree has lingered on.

We'd like to get to a point where Espree is basically just a thin layer on top of an already-existing parser. As it turns out, Acorn already has the hooks we need to do that. So, in the interest of supporting an already-existing and well-supported parser, I'll start working on moving Espree to be built on top of Acorn. The intent is to provide the exact same interface that Espree currently has, effectively turning Espree into a translation layer between Acorn and Esprima outputs.

Things to get working:

  • Comment attachment
  • arrowFunctions
  • binaryLiterals
  • blockBindings
  • classes
  • defaultParams
  • destructuring
  • experimentalObjectRestSpread
  • forOf
  • generators
  • globalReturn
  • jsx
  • modules
  • newTarget
  • objectLiteralComputedProperties
  • objectLiteralDuplicateProperties
  • objectLiteralShorthandMethods
  • objectLiteralShorthandProperties
  • octalLiterals
  • regexUFlag
  • regexYFlag
  • restParams
  • spread
  • superInFunctions (removed)
  • templateStrings
  • unicodeCodePointEscapes
  • All tests running as modules
  • Ecma Features mix
  • Tokenize
@xjamundx
Copy link
Contributor

👏 Cool, let me know if you need any specific help!

@nzakas
Copy link
Member Author

nzakas commented Sep 15, 2015

I'm going to try to dig into this this week and hopefully will have some tasks you can chip in on. In the meantime, take a look at the acorn branch to get an idea of what's involved.

@nzakas
Copy link
Member Author

nzakas commented Oct 7, 2015

Working on this.

@nzakas
Copy link
Member Author

nzakas commented Oct 7, 2015

As a side note, I'm going to remove superInFunctions. It doesn't actually make sense anymore since the spec changed such that super can't really be used in functions anymore.

@nzakas
Copy link
Member Author

nzakas commented Oct 8, 2015

Getting close, waiting on a few compat fixes:

estree/estree#98
acornjs/acorn#326
acornjs/acorn-jsx#28
acornjs/acorn#325

@dead-claudia
Copy link

Should this be noted in the README? It doesn't seem to be up to date in light of this bug.

@xjamundx
Copy link
Contributor

xjamundx commented Oct 8, 2015

whoa @nzakas you got that done super fast

@nzakas
Copy link
Member Author

nzakas commented Oct 8, 2015

The readme will be updated when the work complete.

@hzoo
Copy link
Member

hzoo commented Oct 9, 2015

effectively turning Espree into a translation layer between Acorn and Esprima outputs.

So we should be able to merge anything in https://github.com/babel/acorn-to-esprima so that babel-eslint/babel-jscs only deals with flow/es7 stuff?

@nzakas
Copy link
Member Author

nzakas commented Oct 9, 2015

I'm not sure what you mean, can you explain some more?

@nzakas
Copy link
Member Author

nzakas commented Oct 9, 2015

@xjamundx I had done some early work a while back, and @RReverser did a bunch as well. I just started from where we left off and fixed failing tests. :)

@hzoo
Copy link
Member

hzoo commented Oct 9, 2015

I was thinking we could use espree in babel-eslint if we could move the functionality there. Like for the template string issues you'll probably find https://github.com/babel/acorn-to-esprima/blob/master/src/convertTemplateType.js.

https://github.com/eslint/espree/blob/b8576c6260401b52b392e0df70932dd0c4ccb9d7/espree.js#L351 is from https://github.com/babel/acorn-to-esprima/blob/master/src/toToken.js

I guess I thought espree could use acorn-to-esprima methods so it's a single place to make updates but I guess not?

@nzakas
Copy link
Member Author

nzakas commented Oct 9, 2015

There's definitely some things that useful, but the overall approach we're taking is a bit different. Here's the latest: https://github.com/eslint/espree/blob/issue200/espree.js

In short, we're doing the conversion on-the-fly instead of after the fact, which allows us to more easily mimic what Espree is doing today in terms of ecmaFeatures errors.

@hzoo
Copy link
Member

hzoo commented Oct 9, 2015

Ok cool I see

@nzakas
Copy link
Member Author

nzakas commented Oct 13, 2015

Basically done with this. Now just waiting for the various issues/PRs on Acorn and Acorn-JSX to land to fix the remaining broken tests.

nzakas added a commit that referenced this issue Oct 15, 2015
nzakas added a commit that referenced this issue Oct 27, 2015
@nzakas
Copy link
Member Author

nzakas commented Oct 27, 2015

Getting closer on this. A bunch of my changes have been merged into Acorn. There are a couple of edge cases I'm still working on, mostly having to do with template strings. Hopefully will have this done in the next couple of weeks.

@nzakas
Copy link
Member Author

nzakas commented Oct 28, 2015

Even closer than I thought after the recent Acorn release. There are just a couple of failing tests left for parse. Then I need to look at tokenize().

@nzakas
Copy link
Member Author

nzakas commented Nov 3, 2015

Just waiting for one bug fix from Acorn: acornjs/acorn#345

nzakas added a commit that referenced this issue Nov 3, 2015
nzakas added a commit that referenced this issue Nov 4, 2015
@nzakas
Copy link
Member Author

nzakas commented Nov 5, 2015

Working through some perf issues. As it is, this is slower than existing Espree. I've been working on cutting the time down and I'm pretty close.

With Espree 2.x:

CPU Speed is 3093 with multiplier 7500000
Performance Run #1:  2542.365766ms
Performance Run #2:  2522.98052ms
Performance Run #3:  2545.898633ms
Performance Run #4:  2531.269895ms
Performance Run #5:  2561.971175ms
Performance budget exceeded: 2542.365766ms (limit: 2424.8302618816683ms)

With Espree 3.x:

CPU Speed is 3093 with multiplier 7500000
Performance Run #1:  2677.714532ms
Performance Run #2:  2671.42152ms
Performance Run #3:  2687.464265ms
Performance Run #4:  2730.368188ms
Performance Run #5:  2718.3307489999997ms
Performance budget exceeded: 2687.464265ms (limit: 2424.8302618816683ms)

nzakas added a commit that referenced this issue Nov 30, 2015
@nzakas nzakas closed this as completed in 94d051f Dec 1, 2015
nzakas added a commit that referenced this issue Dec 1, 2015
Breaking: Switch to Acorn (fixes #200)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants