Upgrade: acorn 6, acorn-jsx 5, and istanbul - #391
Conversation
I encountered it in #387 , so I upgraded |
| }, code); | ||
|
|
||
| // TODO: remove global state. | ||
| commentAttachment.reset(); |
There was a problem hiding this comment.
At this point, since we don't use comment attachment in ESLint, I wonder if we should actually remove the comment attachment option in the future? I don't think it adds much value as of right now and adds some maintenance burden.
There was a problem hiding this comment.
I agree. We can remove the comment attachment logic in the future.
But this PR focuses on upgrading acorn since it's a breaking change that removes attarchComment option.
There was a problem hiding this comment.
Yes, sorry I wasn't clear. Wasn't trying to suggest we change that in this PR!
| * @returns {number} normalized ECMAScript version | ||
| */ | ||
| function normalizeEcmaVersion(ecmaVersion) { | ||
| if (typeof ecmaVersion === "number") { |
There was a problem hiding this comment.
I know this is the current behavior, but I wonder if we should print a warning or something in the case that ecmaVersion is not a number? It seems like it could be hard for a user to debug this if they had set the value to "9" and it kept parsing with ES5.
| instance.extend("finishNodeAt", wrapFinishNode); | ||
|
|
||
| instance.extend("next", function(next) { | ||
| return /** @this acorn.Parser */ function() { |
There was a problem hiding this comment.
Is this behavior already taken care of in the super class's method?
There was a problem hiding this comment.
Yes. This override exists to address acornjs/acorn#363 and it has been fixed.
kaicataldo
left a comment
kaicataldo
left a comment
There was a problem hiding this comment.
Have just a few questions. Thanks for working this!
|
friendly ping @kaicataldo , does you comments addressed? /cc. @mysticatea seems there's a conflict need to resolve. |
|
Thank you! |
4 participants
This PR upgrades some dependencies:
acornto 6.0.2 ... This major version has changed the plugin system drastically.acorn-jsxto 5.0.0 to use new plugin system.istanbultonyc, the official successor ofistanbul, to measure coverage for ES2015 syntax (I encountered some conversion errors).This PR moves the
acornplugin part to./lib/espree.jsfrom./espree.js. (...and I deleted some unused logics.) New plugin system is class-based, so I use symbols to store private stuff in order to vaoid confliction with acorn's private stuff.Probably this upgrade will fix eslint/eslint#10623.
Fixes #390.