-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[ES6] Error when parenthesis string is envolved by parenthesis #1001
Comments
Are you using the uglify harmony branch? |
@kzc yep |
Could you put [ES6] in the issue title? |
Someone beat me to this. :) Cc: @fabiosantoscode Dummy Userscript extraction that doesn't really do anything other than produce the minification issue reported here (function () {
// ==UserScript==
// @name RFC 2606§3 - UglifyJS2 (Harmony) Unit Test
// @namespace https://openuserjs.org/users/Marti
// @decription Tests harmony minification
// @copyright 2016+, Marti Martz (https://openuserjs.org/users/Marti)
// @license (CC); http://creativecommons.org/licenses/by-nc-sa/3.0/
// @license GPL version 3 or any later version; http://www.gnu.org/copyleft/gpl.html
// @version 2016.03.09
// @icon https://www.gravatar.com/avatar/7ff58eb098c23feafa72e0b4cd13f396?r=G&s=48&default=identicon
// @include http://www.example.com/*
// @include http://www.example.net/*
// @include http://www.example.org/*
// @grant none
// ==/UserScript==
function (col) {
return col.slice(0, col.indexOf('('));
}
})(); MINIFICATION WARNING (harmony):
message: Unexpected token: punc (()
installName: Marti/RFC_2606§3_-_UglifyJS2_(Harmony)_Unit_Test.user.js
line: 20 col: 11 pos: 742 We see a lot of these... function UserscriptExtraction(col) {
return col.slice(0, col.indexOf('('));
} MINIFICATION WARNING (harmony):
message: Unexpected token punc «)», expected arrow «=>»
installName: Marti/RFC_2606§3_-_UglifyJS2_(Harmony)_Unit_Test.user.js
line: 21 col: 40 pos: 830 ... modified to a named function from above to produce the arrow message. Actual log excerpts:2016-03-09 10:23:47.724 +00:00: MINIFICATION WARNING (harmony):
message: Unexpected token punc «)», expected arrow «=>»
installName: vBm/Omerta_Beyond.user.js
line: 970 col: 40 pos: 34036 ... taken from https://openuserjs.org/scripts/vBm/Omerta_Beyond/source and our logs. (NOTE: Line numbers could change/disappear if published/updated date is newer) function UserscriptExtraction() {
var strFecha = getElementsByClass("start")[0].innerHTML;
strFecha = strFecha.substring(strFecha.indexOf('(')+1, strFecha.indexOf(')'));
} 2016-03-06 20:57:08.094 +00:00: MINIFICATION WARNING (harmony):
message: Unexpected token operator «+», expected arrow «=>»
installName: Amynka/CCcompact.user.js
line: 528 col: 55 pos: 16404 ... taken from https://openuserjs.org/scripts/Amynka/CCcompact/source and our logs. (NOTE: Line numbers could change/disappear if published/updated date is newer) 2016-03-07 02:05:33.281 +00:00: MINIFICATION WARNING (harmony):
message: Unexpected token punc «[», expected arrow «=>»
installName: open2chExtender/open2chYouVideo.user.js
line: 132 col: 483 pos: 38238 ... taken from https://openuserjs.org/scripts/open2chExtender/open2chYouVideo/source and our logs. (NOTE: Line numbers could change/disappear if published/updated date is newer... this script is partially pre-minified too so it's difficult to extract the snippet) e.g. the issue seems to show up as more than just parens too... sometimes after |
@mishoo and @fabiosantoscode Example:
https://openuserjs.org/src/scripts/vBm/Omerta_Beyond.min.user.js#
GET /src/scripts/vBm/Omerta_Beyond.min.user.js HTTP/1.1
Host: openuserjs.org
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:42.0) Gecko/20100101 SeaMonkey/2.39
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate
DNT: 1
Referer: https://openuserjs.org/scripts/vBm/Omerta_Beyond/source
Cookie: *clipped*
Connection: keep-alive
HTTP/1.1 200 OK
X-Powered-By: Express
Strict-Transport-Security: max-age=31536000000; includeSubDomains
Content-Type: text/javascript; charset=UTF-8
Warning: 199 openuserjs.org MINIFICATION WARNING (harmony): Unexpected token punc =?utf-8?Q?=C2=AB=29=C2=BB=2C?= expected arrow =?utf-8?Q?=C2=AB=3D=3E=C2=BB?= line: 970 col: 40 pos: 34036
Vary: Accept-Encoding
Content-Encoding: gzip
Date: Wed, 09 Mar 2016 23:13:38 GMT
Connection: keep-alive
Transfer-Encoding: chunked
I've also added a Minified Source button dropdown to script homepages for more ease of checking Example:
|
… present * Appears that the `«` and the `»` aren't in a *UglifyJS2* code search... so do this on our end so it's more readable Applies to OpenUserJS#432 and post fix for OpenUserJS#923 ... also suggested at mishoo/UglifyJS#1001 (comment)
@viclm |
(function () {
// ...
function (col) {
return col.slice(0, col.indexOf('('));
}
})(); This is not a valid program, so it's not surprising that it fails to parse. (The inner function is in statement position, thus requiring a name) |
===:
... Just because it's not a real program doesn't negate the parsing error in UglifyJS2... it is syntactically correct just doesn't do anything... it's an example if you would please reread what was posted and requoted ... technically it's dead code without the name but still a 100% valid function expression ... just think if it didn't have my typical IIFE around it... e.g. ignore that as it's just a layer of abstraction... however...
The parsing failure is denoted by @viclm and called out with a fix... I'm just here to confirm that he's right that there is definitely an issue. I can bombard this projects issue tracker with the dozens, if not hundreds, of parsing errors this project has (we have ~300+ so far this month unsorted and this is a conservative count) but I thought I might be more constructive by actually isolating as much as I can for this project in order to improve it... filtering and condensing logs when needed including common mistakes that may generate a parsing error, etc. We have a growing, large, community over at our service so it would be nice to be able to offer suggestions, confirm accuracy, etc. to our dependency friends out there. There are multiple errors with parsing in this project that I haven't posted yet including Point of the matter is that somewhere in the ES6 code it's interfering with with ES5. Cc: @fabiosantoscode Our first logged instance of this issue occurred on |
acorn@3.0.4 agrees with @rvanvelzen's assessment:
|
@kzc and @rvanvelzen You are both missing the point of the snippet... while it may be called out as an exception that doesn't negate @viclm's report... this project has a major flaw right now with defaulting to "expected arrow" messages. I always test my reduced case snippets and the error should be something along the lines of: /*
Exception: SyntaxError: function statement requires a name
@Scratchpad/1:12
*/ not ... MINIFICATION WARNING (harmony):
message: Unexpected token: punc (()
installName: Marti/RFC_2606§3_-_UglifyJS2_(Harmony)_Unit_Test.user.js
line: 20 col: 11 pos: 742 ... so presuming that acorn is the deriving backend here... clearly it is incomplete on the error message for that particular code snippet that was thrown together as a reduced case scenario.... anyhow... this is the end of nit picking on this particular sample snippet as it produces no effective resolution for the base issue here... and why I named it (you both are clearly demonstrating that you missed that in the initial post above too... see "We see a lot of these..." section)... e.g. please don't assume that I'm stupider than you are please... this project has been notified of an issue and I would hope that one of the maintainers would be willing to address the subject of the parens and get the patch verified that @viclm did... it works here for the hundreds of errors that are in our logs from this project. I can understand if those here don't want to fix a known issue and I also understand that those here may not understand common issue tracking as I've been doing this for over three decades... quite possibly longer than most have been alive here but I won't presume that is true in all cases... so please try to concentrate on constructive issues rather than assert ones programming ego on a single code snippet. :) Secondly the parens issue is 100% reproducible which is what this issue is about... and there is more than just parens. It would be great not to have to switch to a different package that is willing to properly address issues... I am starting to realize how our other contributors don't like the idea of altering the code because at the very least it's buggy at times... this is putting it nicely... they didn't put it nicely. I would hope that this project wouldn't want to burn me or us... we represent a larger community group of users than all of node. What's left for maintainers to do constructively:
I'll reference it in another one/set with the |
@Martii You're not helping your cause with such long-winded and insulting diatribes. We're all volunteering our time here. We're happy to receive succinct bug reports and pull requests. But if you want to take your javascript minification business elsewhere, that's up to you. |
So are we... and we've bent over backwards to promote this project and suppress the negative connotations in the global community. e.g. change perceptions.
Change your perception please as it's incorrect as well as snarky (flaming, trolling, misguided, etc.)
Prefer the term of completeness instead of being lazy like some want and expect. My colleagues in the office have just read this and they concur with the assessment. We are handing this data as a courtesy... perhaps some equal return courtesy would be nice... including reading everything and not falling back on the TLDR aspect of some.
That's great... perhaps referencing back to the PR would have been prudent as I just did that above... that's why CONTRIBUTING.md exists on GH. :) I don't expect full a bureaucracy but to alleviate some miscommunications it's a wise idea to do.
It's actually up to this project and how it will be rated from a global standpoint. e.g. this projects reputation is on the line as the aphorism goes and so far at least one is burning one of the few advocates (me) of it. This I will be blunt on... that's a stupid and arrogant move. Anyhow... rather than me receiving a defensive posture and drama which seems to be this issues preferred responsed form of communication so far... how about confirming that is the correct patch for some constructive feedback? Is it? Do you know? One more time in case it was missed for the third time... is that the correct patch? (if no one decides to answer this simple question I won't be responding anymore to this issue as it's a confirmed issue on our end of several million users in the Userscript and node communities. :) @fabiosantoscode |
I've merged @viclm's PR, which should resolve the original issue. For issues other than the eager parsing of arrow functions, please open a new issue. |
@Martii Please stop hijacking threads with useless commentary. |
Thank you @rvanvelzen for the confirmation. All three reported Userscripts now show as being minified without errors. (cross-section check of all March 2016 logged errors ... the 300ish report above... shows as successful now) :) Awesome modified patch @viclm ... thank you for your patience and effort. Closed by 6780d09 |
I completely missed the development of this conversation, and just wrote an unnecessary fix lol :) thanks @viclm for fixing the bug I introduced. Sorry about the oversight. Not mentioning any names in specific: Let's get along, everyone ❤️ there's no need to be aggressive. I believe we're all working towards the same goal. Have a beautiful day! |
('(')
Unexpected token eof «undefined», expected arrow «=>»
The text was updated successfully, but these errors were encountered: