deprecated.property raise error when object is empty

[snowyu]

I do not wanna to check the property of the options whether exists, why not do this in depd?

deprecated.property(options, "prop")
//if isObject(options) && (options.prop) deprecated.property(options, "prop")

[dougwilson]

Well, this is made to deprecate your API, so the premise is you would know what your own API looks like.

But I see you are deprecating the accessing/setting of a property on a general options object. I can certainly think about the support. I'd be interested in seeing how you're trying to use it so I can make sure I come up with the correct solution, as just making it not error for the above may seem to work, but there could possibly be a new kind of API that's even easier to do what you're trying.

If you cannot link to an example since it's not open source, you can always paste example code in here. Basically, to me, it seems weird to want to deprecate a property on an options object, but perhaps you use it in a different way than I ever have.

[dougwilson]

P.S. The reason that function requires the property to exist, is because for us to properly re-define it, we have to know if it's configurable and if it's enumerable.

[snowyu]

ok , because the object is optional argument. here it is: https://github.com/snowyu/level-subkey/blob/master/shell.js#L79

[dougwilson]

Ok. I think you are using the API incorrectly. To me, it looks like what you want for that line is actually this:

if (opts.prefix) deprecate('prefix option, use `path` instead.')

[dougwilson]

deprecate.property does not display a message to the user, rather it sets up the property to print a deprecation when the user eventually tries to get or set that property. Your code when immediately accesses the property on the next line, making it look like you are using the right API :)

[snowyu]

Got it. the js is very flexible script language, I can not compel developer to use a specified Options object like java. IMO most properties on javascript are just in a simple json object. so deprecate.property seldom useful. On java it is useful enough.

btw, I am too lazy to type the check code in deprecate statement repeatedly.

[dougwilson]

deprecate.property is to deprecate properties your users are going to access, not properties in objects that users are going to pass into your code. I hope that makes it clear what the difference is.

In the code you linked to. if you removed the line

if (opts.prefix && !opts.path) opts.path = opts.prefix

then you would see how the deprecation message from deprecate.property would no longer appear, because it's the wrong API for what you're trying to do is all. To deprecate things on incoming objects you just use the deprecate(msg) API and not the others.

[dougwilson]

Basically, your code should look like this:

if (opts.prefix) {
  deprecate('prefix option, use `path` instead.')
  if (!opts.path) opts.path = opts.prefix
}

[snowyu]

I have wrapped it with this:

  function deprecatedPrefixOption(options) {
    if (options.prefix) {
      deprecate('prefix option, use `path` instead.')
      if (!options.path) options.path = options.prefix
      delete options.prefix
    }
  }

But this can be extracted as a new feature to do, what is a perfect name?

function deprecate.assignProperty(object, deprecatedProp, currentProp) {
    if (object[deprecatedProp]) {
      deprecate(deprecatedProp+'property, use `'+currentProp+'` instead.')
      if (!object[currentProp]) object[currentProp] = object[deprecatedProp]
      delete object[deprecatedProp]
    }
}

[dougwilson]

That API is out of the scope of this module, unfortunately, as it's extremely opinionated in how you're supposed to use objects as options, including the fact that all you're doing is changing the name of a property on an incoming object. Next thing you know, I'll need to extend it because people want the currentProp to depend on the contents of the deprecated property's value, then they'll want to support the deprecated property's value being 0, undefined, or null. This can go on forever...

Your helper also won't function correctly, because the call location of your deprecate function is required to be unique in your file for each instance, otherwise sometimes the warnings may not appear to the user.

[snowyu]

Yes, the assignProperty helper function is just for assigning the current property to a depreacted property. even they do not wanna delete the deprecatedProp. Enhance a good API is not easy thing.

Pardon? your means in the example code, should use this instead of deprecate function.