generated getters/setters for class property assertions break property ownership
This is a:
- [x] Bug Report
- [ ] Feature Request
- [ ] Question
- [ ] Other
Which concerns:
- [x] flow-runtime
- [x] babel-plugin-flow-runtime
- [ ] flow-runtime-validators
- [ ] flow-runtime-mobx
- [ ] flow-config-parser
- [ ] The documentation website
When I trying to clone my object with lodash clone method, it loses values of object properties:
// @flow
import { clone } from 'lodash'
class A {
prop: number
constructor(prop: number) {
this.prop = prop
}
}
test('cloning of typed objects', () => {
const a = new A(42)
const copy = clone(a)
expect(a).not.toBe(copy)
expect(copy.prop).toBe(42)
})
With example above I got a error from jest:
FAIL a.spec.js
✕ cloning of typed objects (6ms)
● cloning of typed objects
expect(received).toBe(expected)
Expected value to be (using ===):
42
Received:
undefined
What is the expected behaviour?
It should returns cloned object with setted properties and works correctly with lodash, because it's popular package from npm.
Which package versions are you using?
- flow-runtime - 0.16
- babel-plugin-flow-runtime - 0.15
- lodash - 4
I was unable to reproduce this. Would you mind putting your package.json, yarn.lock or package-lock.json if any, .babelrc, and .flowconfig into a gist?
@jedwards1211, yes, I move part of my project to repository https://github.com/RuBAN-GT/flow-sample.
To see this error you should run the command:
yarn run test
Thank you for your help!
Thanks for the code @RuBAN-GT, I was able to reproduce it!
The reason this is happening is that babel-plugin-flow-runtime essentially calls Object.defineProperty(A.prototype, 'prop', {...}) to establish a getter and setter that perform runtime type checks.
But lodash.cloneDeep only Because the property is defined on A.prototype rather than an instance of A, it doesn't show up as an own property of the instance of A when lodash is cloning it.
Until we fix this you could work around this by using cloneDeepWith on any object tree that contains instances of A, like so:
function customizer(value) {
if (value instanceof A) {
return new A(value.prop)
}
}
var clonedStuff = cloneDeepWith(stuff, customizer);
So @phpnode I think babel-plugin-flow-runtime should be calling Object.defineProperty(this, 'prop', {...}) in A's constructor rather than on A.prototype. Would you agree?
@phpnode because this bug causes the prototype to own class properties instead of the class itself it could cause quite a few other issues as well.
The simplest way to demonstrate this is that Object.keys(new A()) outputs an empty array, rather than ['prop'] as it should.
Yes I think so, this seems to be a consequence of using decorators for this. I think at this point we should switch to doing this work in the constructor and drop the requirement for decorators entirely (we can still support their use, but we don't need em). I also noticed that we don't even do property validation when a class property doesn't have an initial value, which is a slight oversight!
Yeah that's true, I can't think of a way to do it with property decorators either. I had a bit of trouble piecing together how all the resulting code gets generated. Any tips on where I should look?
@jedwards1211 yeah it's a little bit tricky / messy, this particular transform happens in https://github.com/codemix/flow-runtime/blob/master/packages/babel-plugin-flow-runtime/src/transformVisitors.js#L789