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

remove 'Number' from public property names #5

Closed
capaj opened this issue Oct 19, 2015 · 7 comments
Closed

remove 'Number' from public property names #5

capaj opened this issue Oct 19, 2015 · 7 comments

Comments

@capaj
Copy link

capaj commented Oct 19, 2015

It is not a good practice to indicate a property type in it's name. If you want to specify it, JSDOC is much better place to do it.
I have been using stacktrace recently in conjunction with source-map from Mozilla and I had to remap stackframe objects to fit

{
  line: 2,
  column: 28
}

because that is a kind of object that they expect. I think it would make sense to unify the naming conventions in Mozilla's favour. What do you think @oliversalzburg ?

@oliversalzburg
Copy link
Member

I'm not quite following. Are you referring to lineNumber and columnNumber? How are the properties creating problems?

I don't think the names were chosen to indicate the type of the variable, but I'm definitely not opposed to changing the names, if they cause a problem.

@capaj
Copy link
Author

capaj commented Oct 19, 2015

@oliversalzburg yes, those two properties are exactly what I was referring to. It's not a problem, as much as it is inconvenient.
It would be easier to use with mozilla's project if the property names would just line and column

@oliversalzburg
Copy link
Member

@eriwen What's your take on this?

This would be a breaking change, as dependent projects could be referencing the lineNumber and columnNumber properties.

@capaj Would convenience getters that map line to lineNumber and column to columnNumber also be a valid solution?

@capaj
Copy link
Author

capaj commented Oct 19, 2015

@oliversalzburg getters would solve it for me.
I would also be okay with waiting until the next major version. Whichever you prefer.

@oliversalzburg
Copy link
Member

@capaj Just to make sure, I assume your use case isn't covered by stacktrace-gps?

@capaj
Copy link
Author

capaj commented Oct 19, 2015

@oliversalzburg I am actually using it to get the actual line/column of a location where error was thrown. I will probably use it-it does exactly what I need. Once I figure out how to get source maps out of SystemJS

@eriwen
Copy link
Member

eriwen commented Oct 19, 2015

The property names are intended to be as similar to Gecko and V8 Error stack representations.

A getter would work, but stackframe is intended for use even in browsers that don't support ES5 getters. We could provide that and a polyfill, but that would significantly impact the library size.

We could perhaps add aliases such that line and column retrieve those values if this especially painful for some reason. I understand this makes the code a little more verbose but this is in the interest of consistency and clarity.

@eriwen eriwen closed this as completed Nov 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants