Skip to content

Instantly share code, notes, and snippets.

@CrypticSwarm
Created August 8, 2012 02:45
Show Gist options
  • Save CrypticSwarm/3291607 to your computer and use it in GitHub Desktop.
Save CrypticSwarm/3291607 to your computer and use it in GitHub Desktop.
Showcases a subtle problem that can occur due to getters.
var optimist = require('optimist')
optimist.demand('color')
optimist.demand('size')
optimist.argv
var optimist = require('optimist')
optimist.demand('color')
optimist.demand('size').argv
@CrypticSwarm
Copy link
Author

Assume optimist is installed.

Run both of the following files with
node [filename]

node broken.js
exits without any messages.

node works.js
exits with the following error message:

Options:
  --color  [required]
  --size   [required]

Missing required arguments: color, size

This is due to the fact that a options parser is automatically created:
https://github.com/substack/node-optimist/blob/master/index.js#L12

Then all of the values are copied onto Argv:
https://github.com/substack/node-optimist/blob/master/index.js#L14

Which is assigned to module.exports
https://github.com/substack/node-optimist/blob/master/index.js#L19

Which includes the argv property which is a getter.
https://github.com/substack/node-optimist/blob/master/index.js#L300

So the value at the time of the copy is used rather than copying the getter.

In broken.js we use the top level object which has the copied value.

In works.js it returns the actual instance rather than the copied values so it has the actual getter attached.

@rwaldron
Copy link

rwaldron commented Aug 8, 2012

If I understand the issue, the correct way to handle this is to get the source property descriptor and use it to define the new "copied" property with defineProperty. I usd that approach to create a "real clone" utility

@tj
Copy link

tj commented Aug 8, 2012

I've seen effectively 0 valid use-cases for accessors. Things like ctx.fillStyle= are just silly

@tj
Copy link

tj commented Aug 8, 2012

plus .argv could easily just be .argv() here, I'm not bashing that API, I've used accessors plenty too, but IMO a language shouldn't even have them unless there's really a compelling reason, that's something I have yet to really see.

@CrypticSwarm
Copy link
Author

@rwldrn Agreed. However, it makes another thing that people need to keep in their minds if they copy something. Having a function (assuming it was bound to the correct object) that returned a value you could copy regularly. Not really arguing for or against, but this was something that made me do a double take when someone asked me about this situation.

Proxies will probably also have similar oddities that show up when people start making more use of them.

My thoughts are if you start using a language feature make sure you are aware of any nuances that might creep in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment