Regression between Acorn 1.0.3 and 1.1.0 in jsdom usage

[domenic]

jsdom/jsdom#1131 Hard to pin this down to an isolated test case, and running the jsdom tests that trigger it requires a bit of setup (install Java for selenium, basically). Our usage of acorn is contained in https://github.com/tmpvar/jsdom/blob/master/lib/jsdom/vm-shim.js, and it might be easier to try to spot what broke by code inspection.

Nothing obvious to me from looking at the acorn commit logs. I tried to do a manual bisect, but apparently the version in git is not the version in npm, and I tried running prepublish but it fails on Windows.

Anyway, thought you guys would want to know about this regression within the same major version, even if I can't isolate it that well. I'm hoping it's easy for you to spot what's going on, but if you can't do so easily, then I'll dig into it eventually.

[domenic]

Seems to be that acorn broke the acorn-globals package, so that's the code to inspect. https://github.com/ForbesLindesay/acorn-globals

[sebmck]

Could be 4d58b34. Looks like the type of AssignmentExpression and AssignmentPattern in node.left was changed from Expression to Pattern.

[sebmck]

Confirmed that it's something to do with assignments.

> require("acorn-globals")("foo = 'bar';");
[]

[marijnh]

Sorry! I had somehow convinced myself that the changes I made to make the walker properly walk ES6 patterns were backwards compatible, at least for ES5-related use. But they weren't, precisely in this case -- the left-hand-side of an assignment used to be walked as an Expression, but is now walked as a Pattern. This is usually a good thing, but it is probably what broke your code.

[sebmck]

It doesn't look like it's used in acorn-global so it might be something else.

[marijnh]

Walking a variable expression as a Pattern rather than an Expression will make it not match the Identifier rule (https://github.com/ForbesLindesay/acorn-globals/blob/master/index.js#L123), but rather the VariablePattern rule. So I do believe this is the issue.

[ForbesLindesay]

Can you deprecate that version and publish a new version with that change reverted please? Then re-release as 2.0.0 if the behaviour is correct?

[domenic]

Glad you guys were able to figure this out!! +1 to @ForbesLindesay's plan.

[marijnh]

Well, I made a mess and pushed two bogus versions missing stuff from github master (don't release at the end of the day, I guess), but I believe I've also successfully deprecated the offending version and pushed proper versions 1.2.1 and 2.0.1.

[domenic]

Thank you so much!