argOverProp - options `null` should be treated as undefined ?

[amelon]

I'm not sure it is a bug, but actually I don't know how to workaround this issue.

I've the following code:

const stampit = require('@stamp/it')
const {argOverProp} = require('@stamp/arg-over-prop')

const Course = stampit()
  .compose(argOverProp({
    _id: null,
  }))

  console.log(
    Course() // works
  )
  console.log(
    Course(undefined) // works
  )
  console.log(
    Course(null) // throws Error
  )

      TypeError: Cannot read property '_id' of null

      at Object.initializer (node_modules/@stamp/arg-over-prop/index.js:12:51)
      at Stamp (node_modules/@stamp/compose/index.js:34:41)
      at Suite.Object.<anonymous>.describe (lib/test_argoverprop/aop.test.js:29:3)
      at Object.<anonymous> (lib/test_argoverprop/aop.test.js:27:1)

Any help is welcome.

[koresar]

TL;DR: In a common JavaScript convention the null value is treated as "defined but missing" value.

The code which processes your Course stamp argument is here: https://github.com/stampit-org/stamp/blob/c1813f52915cbece4c905b1e6b9fed8ba6fcc7f1/packages/compose/index.js#L28 See the options argument.

Here is a line from the specification:

Note that if no options object is passed to the factory function, an empty object will be passed to initializers.

In JS the undefined value is treated as "not passed". But null is treated as "passed but no data".

I hope that answers your question.

If you have an idea how to improve the development experience here - feel free to suggest.

[koresar]

On the other hand we can modify arg-over-prop to gracefully handle nulls. https://github.com/stampit-org/stamp/blob/c1813f52915cbece4c905b1e6b9fed8ba6fcc7f1/packages/arg-over-prop/index.js#L7-L17

Here we can change this: incomingValue = opts[key] to be that: incomingValue = (opts || {})[key]

What do you think?

[amelon]

thanks for this clear explanation.

This change will work for my use case. I'm totaly ok :)

[koresar]

@amelon We should take developer experience seriously. I kind of gave it a second thought. :))) I'm fixing arg-over-prop not to throw. Thank you for the issue :)

Going to publish the module v1.0.2 in few minutes as a bugfix.

[koresar]

v1.0.2 was just published https://www.npmjs.com/package/@stamp/arg-over-prop