widdershin / cycle-restart Goto Github PK

The cycle-history driver adds extra properties for accessing history functions

https://github.com/cyclejs/history/blob/master/src/makeHistoryDriver.js#L95-L96

I find that these are stripped off from sources.

For reference I'm using this code

https://github.com/OpenDirective/brian/blob/master/client/src/js/runHot.js

I guess I'm assuming the history driver is restart able.

In my haste I took a shortcut that means if you call DOM.select(...).events(...) more than once it won't work! There's a failing test for this on the multiple-streams branch.

With the new Cycle diversity release candidates, the @cycle/dom driver source has a select function on its prototype, then I think the wrapSource function strips all the prototype functions out when it does this code:

Object.keys(source).forEach(function (key) {
  var value = source[key];

  if (key === 'dispose') {
    returnValue[key] = makeDispose({ streams: streams }, value);
  } else if (typeof value === 'function') {
    returnValue[key] = wrapSourceFunction({ streams: streams, addLogEntry: addLogEntry }, key, value, returnValue, scope);
  } else {
    returnValue[key] = value;
  }
});

As object.keys only loops over properties, not the prototype, it never adds the select function to the return value object.

I can put a pull request if you'd like to fix this. Using a for...in loop should do the trick.

Given the following code in an intent:

  const assetUpdate$ = id$
    .flatMapLatest(id => Observable
      .merge(
        DOM
          .select('.title')
          .events('input')
          .map((e) => ({
            id,
            title: e.target.value,
          }))
      )
    )
    .do(() => console.log(1))

When using cycle-restart:

1: id$ is fired once
2: press key in ".title" field
3: "1" is printed once
2: id$ is fired 4 times
2: press key in ".title" field
3: "1" is printed 5 times

What's going on?

Re-subscribing to the same DOM element seem to duplicate stuff?

systemjs-hot-reloader provides a __reload hook for each module, which can be used like so:

var state = [];
export function addItem(text) {
  state.push(text);
}
export function removeItem(text) {
  state = state.filter(item => item != text)
}
export function getItems() {
  return state;
}
window.addItem = addItem;
window.removeItem = removeItem;
window.getItems = getItems;

export function __reload(deletedModule){
  console.log('__reload');
  state = deletedModule.getItems();
  console.log('  restored items: ', state);
}

Example taken from alexisvincent/systemjs-hot-reloader#23 (comment)

Do you have any ideas how to make cycle-restart work given this model?

Just started using Cycle.JS and cycle-restart so the following may be just my ignorance :)

When using isolate in an app, no changes to the DOM code are loaded visibly by cycle-restart. If isolate is not used then the issue does not occur and updates to the code are reflected visibly after restart.

Digging into the code it seems that restartable explicitly does not wrap isolate methods (though I couldn't find the rationale for this in source / commit history).

Meanwhile, the isolateSource method on the DOM driver (v22.3.0) returns a brand new object. This new object is therefore never wrapped and its stream of events not recorded by cycle-restart for replay later.

Commenting out the logic in cycle-restart that excludes wrapping isolate functions solves the issue with no noticable side-effects:

// typeof name === 'string' && name.indexOf('isolate') !== -1 

This error causes webpack to ditch the reload:

This happens inside of a certain function that was given to onDispose in restartable, which traces the error either to https://github.com/Widdershin/cycle-restart/blob/master/src/restartable.js#L154 or https://github.com/Widdershin/cycle-restart/blob/master/src/restartable.js#L98.

repro: https://github.com/edge/cyc/tree/restart

Currently, the history api support I've added to cycle-dom will record all of your DOM events, which is a big obvious memory leak.

This should only occur when actually using cycle-restart.

Currently get this error when reloading:

Uncaught ObjectDisposedError: Object has been disposed
Disposable.checkDisposed @ rx.all.js:794
Rx.Subject.addProperties.onNext @ rx.all.js:11743
tick @ index.js:15659

I'm having trouble figuring out why this error occurs during hot-reloading of only DOM?

Any ideas?

Uncaught TypeError: Cannot read property 'onNext' of undefined(anonymous function) @ restartable.js:219
run @ rx.all.js:11214
ScheduledItem.invokeCore @ rx.all.js:980
ScheduledItem.invoke @ rx.all.js:968
Rx.VirtualTimeScheduler.VirtualTimeSchedulerPrototype.start @ rx.all.js:11124
(anonymous function) @ restart.js:64

Going by my project here: https://github.com/wyqydsyq/unicycle

HMR itself seems to be configured fine, if I make changes to a file the HMR client will output that the relevant dependency graph has changed, the HMR update seems to contain the correct data, but when attempting to mount the new code it wipes the DOM underneath my app's mount point.

On the first update from HMR my app's DOM gets wiped completely (the node it was mounted to is left intact but has no children), on the second update I get an error in the console about Cannot read property 'removeChild' of null but my app's DOM gets correctly mounted based on the most recent update. It alternates between these two states.

Initial page load, everything is fine:
http://i.imgur.com/9oxdSdL.png

After first change (added an ! to the end of the text), DOM breaks:
http://i.imgur.com/JqsjyhA.png

After second change (added another ! to the end of the text), DOM is restored based on new code:
http://i.imgur.com/elYuxIF.png

This could very likely be just me doing something stupid, but this issue only surfaced when I updated Cycle and related packages to Unified versions and none of my config tinkering seems to have helped.

Current API:

domDriver.replayHistory(history)
sources.history(): history

Current the format of history is:

{
  '.add': {
    '.click': {stream: new Rx.Subject(), events: [{timestamp, event}]}
  }
}

That data format is almost guaranteed to break when auto-scope lands, but I will cross that bridge when I come to it.

Following the example but using the following command line:

budo index.js:bundle.js -d public -- -t babelify -p browserify-hmr

"Reloading" is never printed:

if (module.hot) {
  module.hot.accept('./src/app', () => {
    console.log('Reloading')
    const app = require('./src/app').default
    restart(app, drivers, { sinks, sources }, isolate)
  })
}

However, in the browser log it prints:

[HMR] Updated modules ["src/components/hub/browser/asset.js", "src/components/hub/browser/grid.js", "src/components/hub/browser/index.js", "src/components/hub/index.js", "src/app.js"]

Any ideas what I might be doing wrong?

Running cycle restart with a driver that gets by char update from text fields really slow things down.

Does operators like debounce, throttle and the like break hot reloading? These are kind of hidden side effects so I could understand if that is the case?

I'm asking because I've had an issue where I had to move a debounce into a driver in order to have hot-reloading working again. Though I'm unsure whether this is because of some other bug?

Currently cycle-restart is unpredictable when using Cycle Unified. After updating one of my projects where everything previously worked flawlessly to use Unified I noticed a few drivers not working as expected or not at all when being restarted. To be considered compatible with Cycle Unified, client-side standard drivers (DOM, HTTP, History) should all work out of the box (excluding wrapper configuration e.g. pauseSinksWhileReplaying). I'll be updating this task to track issues relevant to this as I encounter them.

• #62 DOM is alternatingly wiped and restored upon HMR updates

Noticed we now have both rerun and restart. What's the difference? When should on be used over the other?

First, thanks for creating this.

I'm new to the Cycle ecosystem, and is finding it hard to piece together a working universal app with hot module replacement. It will be good to have an example of this.

Using cycle-restart with the latest @cycle/run (3.0.0) I get the following error upon initial load:

Uncaught TypeError: Cannot read property 'Time' of undefined
    at restart.js:154
    at restart.js:173
    at Object.options.path (index.ts:22)
    at __webpack_require__ (bootstrap 06d4da1…:659)
    at fn (bootstrap 06d4da1…:83)
    at Object.<anonymous> (process-update.js:132)
    at __webpack_require__ (bootstrap 06d4da1…:659)
    at bootstrap 06d4da1…:708
    at bootstrap 06d4da1…:708

Having a quick peek into the relevant source, I'm not sure how this code is even intended to work; the call to Cycle() returns the dispose method, but the code seems to be expecting an object with sources, sinks, and run properties:

As you can see in this screenshot from the debugger, when looping over the Time driver everything seems to be undefined:

Any news on supporting cycle-restart with the latest "diversity" iteration of Cycle.js? Perhaps with xstream? Thanks!

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.

This isssue is in cycle repo:
cyclejs/cyclejs#263
The problem is that when just dispose DOM driver sink/sources there maybe some leaks (destory hooks do not fire) after that because vdom is not cleaned up properly.

Currently, DOM updates are the biggest slowdown when replaying. In reality, we only care about the last state of the DOM, so we should skip all but the last DOM update.

As seen in Widdershin/cycle-hot-reloading-example@b5c18b3

Here is a gif that demonstrates the problem

This is because repeated calls to isolate without an explicit scope produce different results.

It seems like either we need to reset the value of isolate when we reload the code, or cycle-restart needs to account for the increase in values.

It seems tricky (with the current implementation of isolate) to reset it on code reload. @staltz @Frikki you folk got any ideas?

cc @TylorS

I believe the npm package needs to have the built lib folder. Webpack is throwing Module not found errors when I try to import it.

Thanks!

I look forward to giving this a try with my example app. Have you tried this with @cycle/history yet?

Funnily enough, I currently support drivers that have selector functions like @cycle/dom and drivers where the provided sources is an observable of observables (like @cycle/dom).

Some good candidates: @cycle/storage and cycle-animation-driver

I'm all excited here putting hot reloading in a certain app. I noticed a bug because the restart code assumes every driver will return (sources), and that's not true. Some drivers only take in sinks (e.g. a console log driver) and some drivers only return sources (e.g. a read-only websocket driver). We need to support those, so we need to add some basic null/undefined check.