Object intersections: broken or underdocumented?

[philikon]

Since upgrade from 0.20.1 to 0.21.0, Flow has been complaining about our use of object intersections (though I'm not sure Flow 0.21.0 actually introduced a behaviour change -- it might just be surfacing a previously swallowed error now).

Indeed I'm not sure that we're using intersections correctly. The docs are a bit sparse, so I took a look at how Flow uses them in its library definitions. A good example seem to be the argument definitions for Node's stream classes. Here they are, formatted slightly differently for readability:

type ReadableStreamOptions = {
  highWaterMark? : number,
  encoding? : ?string,
  objectMode? : boolean
};

type WritableStreamOptions = {
  highWaterMark? : number,
  decodeString? : boolean,
  objectMode? : boolean
};

type DuplexStreamOptions = WritableStreamOptions & ReadableStreamOptions & {
  allowHalfOpen? : boolean,
  readableObjectMode? : boolean,
  writableObjectMode? : boolean
};

It is my understanding that an object of type DuplexStreamOptions will also conform to ReadableStreamOptions and WritableStreamOptions. In fact, that's how the streams.Duplex constructor in Node treats it. Quoting the node docs on this:

options Object Passed to both Writable and Readable constructors. Also has the following fields:
...

So if I have a function that consumes a DuplexStreamOptions, shouldn't it be allowed to access properties from all three sub-types? E.g.:

function hasObjectMode(options: DuplexStreamOptions): boolean {
  return options.objectMode || options.readableObjectMode || options.writableObjectMode;
}

However, Flow isn't happy with that:

stream.js:16
 16: type DuplexStreamOptions = WritableStreamOptions & ReadableStreamOptions & {
                                                        ^^^^^^^^^^^^^^^^^^^^^ property `encoding`. Property not found in
 16: type DuplexStreamOptions = WritableStreamOptions & ReadableStreamOptions & {
                                ^^^^^^^^^^^^^^^^^^^^^ object type

stream.js:16
 16: type DuplexStreamOptions = WritableStreamOptions & ReadableStreamOptions & {
                                                                                ^ property `allowHalfOpen`. Property not found in
 16: type DuplexStreamOptions = WritableStreamOptions & ReadableStreamOptions & {
                                ^^^^^^^^^^^^^^^^^^^^^ object type

stream.js:16
 16: type DuplexStreamOptions = WritableStreamOptions & ReadableStreamOptions & {
                                                                                ^ property `readableObjectMode`. Property not found in
 16: type DuplexStreamOptions = WritableStreamOptions & ReadableStreamOptions & {
                                ^^^^^^^^^^^^^^^^^^^^^ object type

stream.js:16
 16: type DuplexStreamOptions = WritableStreamOptions & ReadableStreamOptions & {
                                                                                ^ property `writableObjectMode`. Property not found in
 16: type DuplexStreamOptions = WritableStreamOptions & ReadableStreamOptions & {
                                ^^^^^^^^^^^^^^^^^^^^^ object type

stream.js:23
 23:   return options.objectMode || options.readableObjectMode || options.writableObjectMode;
                                                                          ^^^^^^^^^^^^^^^^^^ property `writableObjectMode`. Property not found in
 23:   return options.objectMode || options.readableObjectMode || options.writableObjectMode;
                                                                  ^^^^^^^ object type

and I'm not sure how to make Flow happy. Neither this:

function hasObjectMode(options: DuplexStreamOptions): boolean {
  let readableOptions: ReadableStreamOptions = options;
  return readableOptions.objectMode || options.readableObjectMode || options.writableObjectMode;
}

nor this:

function hasSimpleObjectMode(options: ReadableStreamOptions | WritableStreamOptions): boolean {
  return !!options.objectMode;
}

function hasObjectMode(options: DuplexStreamOptions): boolean {
  return hasSimpleObjectMode(options) || options.readableObjectMode || options.writableObjectMode;
}

seems to work.

But it gets weirder. If I simulate the way node would use these types, it all seems to work just fine:

class Readable {
  encoding: ?string;
  constructor(options: ReadableStreamOptions) {
    this.encoding = options.encoding;
  }
}

class Writable {
  decodeString: boolean;
  constructor(options: WritableStreamOptions) {
    this.decodeString = !!options.decodeString;
  }
}

class Duplex {
  readable: Readable;
  writable: Writable;
  allowHalfOpen: boolean;
  constructor(options: DuplexStreamOptions) {
    this.readable = new Readable(options);
    this.writable = new Writable(options);
    this.allowHalfOpen = !!options.allowHalfOpen;
  }
}

I can even make Duplex use properties from all three sub-types without problems:

class Duplex {
  readable: Readable;
  writable: Writable;
  allowHalfOpen: boolean;
  encoding: ?string;
  decodeString: boolean;
  constructor(options: DuplexStreamOptions) {
    this.readable = new Readable(options);
    this.writable = new Writable(options);
    this.allowHalfOpen = !!options.allowHalfOpen;
    this.encoding = options.encoding;
    this.decodeString = !!options.decodeString;
  }
}

BUT: I can't use properties from different subtypes in the same expression:

class Duplex {
  readable: Readable;
  writable: Writable;
  allowHalfOpen: boolean;
  objectMode: boolean;
  constructor(options: DuplexStreamOptions) {
    this.readable = new Readable(options);
    this.writable = new Writable(options);
    this.allowHalfOpen = !!options.allowHalfOpen;
    this.objectMode = options.objectMode || options.readableObjectMode || options.writableObjectMode;
  }
}

fails in the same way as the hasObjectMode example above.

So, are object intersections slightly broken, or am I doing something wrong?

[pasha-mf]

See #1331 -- there are many outstanding issues with intersections right now, but hopefully they'll be addressed soon. It seems to me that in your case the root of the issue is that Flow believes (incorrectly) that an intersected type may be in only one of the subtypes at a time, which is the first bug on that list.

[philikon]

I saw that meta-issue, but didn't quite understand the title of #1327 to be describing my issue. Thanks!

[trodrigues]

I'm having an issue which I believe to be similar, or pretty much the same.

I'm seeing the following error:

lib/entities/entry.js:46
 46: export type EntryCollection = EntryCollectionResponse & {
                                                             ^ property `toPlainObject`. Property not found in
 60:   return wrappedData
              ^^^^^^^^^^^ object type

For this code:

type EntryCollectionResponse = {
  total: number,
  skip: number,
  limit: number,
  items: Array<Entry>,
  includes?: IncludesCollection
}

/**
 * EntryCollection type
 */
export type EntryCollection = EntryCollectionResponse & {
  toPlainObject: () => Object
}

export function wrapEntryCollection (data: EntryCollectionResponse): EntryCollection {
  const wrappedData: EntryCollection = mixinToPlainObject(cloneDeep(data))
  if (wrappedData.includes) {
    mixinLinkGetters(wrappedData)
  }
  return wrappedData
}

However, if I just duplicate all the properties in EntryCollectionResponse inside of EntryCollection, it works fine.

[samwgoldman]

@trodrigues Seems like mixinToPlainObject is a key part of this code, but it's not included. (Edit: also cloneDeep)

[trodrigues]

Ah, sorry, those are just other modules that get imported. cloneDeep is from lodash (so, not typed, and I don't have any typing definitions added for it), and the mixin is fairly straightforward:

export default function mixinToPlainObject (data) {
  return Object.defineProperty(data, 'toPlainObject', {
    enumerable: false,
    configurable: false,
    writable: false,
    value: function () {
      return cloneDeep(this)
    }
  })
}

The wrapEntryCollection method essentially just takes a server response and then adds some methods to it like toPlainObject and adds getters to some properties (via the mixinLinkGetters method, but this part is actually not very relevant to the issue at hand).

So essentially, my plan was to have an EntryCollectionResponse which represents the response received from the server, and then EntryCollection just extends that with the additional stuff that gets added by the wrapper method.

[samwgoldman]

Are there still issues here?

[philikon]

As of 0.22.1, the node streams example works, so that's nice! I'll check with our internal codebase later, but closing this in the meantime.

[philikon]

Confirmed: 0.22.1 is happy about the object intersections in our internal codebase as well. Thank you very much for the fixes!