Make "column"-property of Errors enumerable #1285
Conversation
nknapp
changed the title
|
I have reworded my commit messages and would not consider this work-in-progress anymore. If all tests pass and this PR does not introduce another regression (I have no Safari to test the bug fixed in 20c965c), it could be merged (from my point of view) and it would probably help @nathanboktae a lot if there was a 4.0.7 release including this PR. |
That's what SauceLabs is for right? I'm not seeing right off hand if Travis CI is kicking off tests in SauceLabs |
|
@wycats gave me commit access tonight. I have created a local branch "tmp/stage-4.0.7" and push the changes there. This should run the tests in SauceLabs and show any Safari-problems. |
|
Tests case failing for Safari 8: '97' should === '5': Checking error column". Why should Safari 8 return a completely different column than all other browsers? |
|
Ah, that's why: jquery/esprima#1290 (comment) |
Related to handlebars-lang#1284 The test ensures that the property is there, because it is important to some people.
Fixes handlebars-lang#1284 Appearently, there is a use-case of stringifying the error in order to evaluated its properties on another system. There was a regression from 4.0.5 to 4.0.6 that the column-property of compilation errors was not enumerable anymore in 4.0.6 (due to commit 20c965c) and thus was not included in the output of "JSON.stringify".
|
The column-property is read-only in Safari 8, its value won't change, even with Object.defineProperty. The test case now omits the column-check, if the property is read-only. |
This PR contains a test case for #1284. It verifies that the column-property is included in the compilation error. The test passes. This is not mean to be merged right now, primarily for discussion.