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

Reserved word #82

Closed
colindresj opened this issue Mar 19, 2016 · 4 comments
Closed

Reserved word #82

colindresj opened this issue Mar 19, 2016 · 4 comments

Comments

@colindresj
Copy link

function is a reserved word. I think it would be prudent to deprecate td.function() in favor of some other method name, perhaps td.func().

The way the source is currently written, everything is actually valid. function is being used as an IdentifierName, which has no problem with reserved words:

module.exports =
  function: require('./function')

However if someone were to do something like this, they'd get a syntax error:

const td = require('test-double');
const { function } = td; // raises an exception

I feel like the above will become a fairly common use case, so by changing the method name to something that can be destructured into an Identifier without a reserved word clash would save some headaches:

const td = require('test-double');
const { func } = td; // this works

Of course, if someone wanted to destructure with the current name they could do it:

const { function: func } = td;

But I feel like that is forcing people into doing something for the sake of lexical compatibility instead of expressiveness, which is, I think, the use case for destructed re-assignment.

Anyways, this is just a suggestion. I understand it could introduce some backwards compatibility problems.

@searls
Copy link
Member

searls commented Mar 19, 2016

I'm not opposed to this if we can gather data on people actually experiencing it as a problem. There are scant few cases when someone would ever want to import just one method on the module, so I'm not really convinced that hypothetical concern is going to be people's lived experience.

I did some research and edge case testing when I renamed create to object and function, and was surprised to find that it didn't really cause any issues, despite being a reserved word.

@colindresj
Copy link
Author

Yeah, the only real time I see this becoming a problem is with destructuring. I agree that most people won't grab just one method, but I do see myself, at least, doing something like:

const {
  func,
  when,
  verify,
  matchers,
  reset
} = require('test-double');

const fetch = func();

when(fetch(42)).thenReturn('Jane User');

fetch(42);

verify(fetch(matchers.isA(Number));
assert(explain(fetch).callCount, 1);

reset();

I personally like the readability of just verify, reset, etc. as opposed to td.verify, td.reset. Not sure how many other people feel this way, but I'll leave it up to you to close or keep open this issue.

As a second, less legitimate reason, td.function causes a lot of syntax highlighters to pick up on the word and color it different to other object properties.

@searls
Copy link
Member

searls commented Mar 27, 2016

How would you feel about simply adding func as an alias?

On Mar 26, 2016, at 17:02, JC notifications@github.com wrote:

Yeah, the only real time I see this becoming a problem is with destructuring. I agree that most people won't grab just one method, but I do see myself, at least, doing something like:

const {
func,
when,
verify,
matchers,
reset
} = require('test-double');

const fetch = func();

when(fetch(42)).thenReturn('Jane User');

fetch(42);

verify(fetch(matchers.isA(Number));
assert(explain(fetch).callCount, 1);

reset();
I personally like the readability of just verify, reset, etc. as opposed to td.verify, td.reset. Not sure how many other people feel this way, but I'll leave it up to you to close or keep open this issue.

As a second, less legitimate reason, td.function causes a lot of syntax highlighters to pick up on the word and color it different to other object properties.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub

@colindresj
Copy link
Author

Yeah that sounds like a pretty good idea to me.

On Mar 26, 2016, 20:09 -0400, Justin Searlsnotifications@github.com, wrote:

How would you feel about simply adding func as an alias?

On Mar 26, 2016, at 17:02, JCnotifications@github.comwrote:

Yeah, the only real time I see this becoming a problem is with destructuring. I agree that most people won't grab just one method, but I do see myself, at least, doing something like:

const {
func,
when,
verify,
matchers,
reset
} = require('test-double');

const fetch = func();

when(fetch(42)).thenReturn('Jane User');

fetch(42);

verify(fetch(matchers.isA(Number));
assert(explain(fetch).callCount, 1);

reset();
I personally like the readability of just verify, reset, etc. as opposed to td.verify, td.reset. Not sure how many other people feel this way, but I'll leave it up to you to close or keep open this issue.

As a second, less legitimate reason, td.function causes a lot of syntax highlighters to pick up on the word and color it different to other object properties.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub


You are receiving this because you authored the thread.
Reply to this email directly orview it on GitHub(#82 (comment))

@searls searls closed this as completed in 63fcbeb Apr 3, 2016
searls added a commit that referenced this issue Apr 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants