Restartable compromises symbol-observable interop

[xtianjohns]

When iterating on source properties in restartable, symbol-observable support is compromised.

This means the following code won't work:

import { from } from 'my-stream-library';

export function component( sources ) {
  /* assuming that the DOM driver was wrapped */
  const source$ = sources.DOM.select( '#foo' ).events( 'click' );
  return {
    anything: from( source$ ), // this throws
  };
}

The error is specifically that source$ does not have a property from symbol-observable that is added by xstream for interop with different stream libraries.

I think this is related to #55, but is slightly different since it isn't about a problem with driver methods/props, but with source/sink props.

To test that this can work, just wrap a driver (DOM sources from @cycle/dom are an acceptable candidate), and test that the property exists.

import $$observable from 'symbol-observable';
export function component( sources ) {
  /* assuming that the DOM driver was wrapped */
  const source$ = sources.DOM.select( '#foo' ).events( 'click' );
  console.info( source$[ $$observable ] );
  /* ... */
}

Also, symbol-observable is just one case, this would be true of all properties that relied on symbols or supported custom features on streams. But it's just an example of the broader problem.

Happy to help work on this @Widdershin, and submit the PR, but it looks like you may have started some work already as part of the Cycle Unified milestone? Let me know how I can help, and I'll get on it.

[xtianjohns]

I have a fix for this working locally for my use-case, but it relies on some newer ES functionality. In my specific case we're looking for interop with Symbols, which should only exist in ES2015 anyway, so I'd be okay with a polyfill for some of this functionality.

Without a polyfill, the tests relying on the behavior break. Here's the gist of my potential solution (not the whole thing). Before I commit and submit a PR, is there any more discussion that we should have on this?

function keys (obj) {
  const _keys = [];

  for (const prop in obj) {
    _keys.push(prop);
  }

  // get all of the symbols on the object
  for(const prop of Object.getOwnPropertySymbols(obj) ) { 
    _keys.push(prop);
  }

  // get all of the symbols on the prototype
  for(const prop of Object.getOwnPropertySymbols(Object.getPrototypeOf(obj) || {}) ) {
    _keys.push(prop);
  }

  return _keys;
}

Would appreciate your thoughts when you have time @Widdershin. Also note that breakages don't affect anything in the monorepo, but they will affect CycleJS flavors that use restartable out of the box (like one-fits-all). CC @jvanbruegge

[Widdershin]

Thanks for the bug report and potential fix. The route you're investigating makes a lot of sense, I'd love a pull request if you have the time. Personally I would be fine with cycle-restart only working in modern browsers, since it's a dev tool. Although I guess maybe it might be useful in browsers without native symbols? Personally I'm happy with either requiring users to have greenfields browsers or including a polyfill.

…

On Sat, Dec 16, 2017 at 10:18 AM, Christian Johns ***@***.***> wrote: I have a fix for this working locally for my use-case, but it relies on some newer ES functionality. In my specific case we're looking for interop with Symbols, which should only exist in ES2015 anyway, so I'd be okay with a polyfill for some of this functionality. Without a polyfill, the tests relying on the behavior break. Here's the gist of my potential solution (not the whole thing). Before I commit and submit a PR, is there any more discussion that we should have on this? function keys (obj) { const _keys = []; for (const prop in obj) { _keys.push(prop); } // get all of the symbols on the object for(const prop of Object.getOwnPropertySymbols(obj) ) { _keys.push(prop); } // get all of the symbols on the prototype for(const prop of Object.getOwnPropertySymbols(Object.getPrototypeOf(obj) || {}) ) { _keys.push(prop); } return _keys; } Would appreciate your thoughts when you have time @Widdershin <https://github.com/widdershin>. Also note that breakages don't affect anything in the monorepo, but they will affect CycleJS flavors that use restartable out of the box (like one-fits-all). CC @jvanbruegge <https://github.com/jvanbruegge> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#67 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAYUHTNhmuwtb4l3nQZShks8_Qe4feL0ks5tAuIHgaJpZM4RD6We> .

[xtianjohns]

PR is up @Widdershin, I included a shim. Open to feedback - I tried to keep the diff small.