Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Polymer-redux 1.0 + Polymer 2 hybrid mode doesn't expose state + dispatch is unknown #87

Open
ronnyroeller opened this issue Jun 13, 2017 · 9 comments

Comments

@ronnyroeller
Copy link
Contributor

It would be great if Polymer-redux would work not only with pure Polymer 2 (e.g. ES6 class syntax) but also with Polymer hybrid mode.

Right now, for hybrid element with the PolymerRedux behavior the Redux state isn't available and calling this.dispatch causes the following error:

Uncaught TypeError: this.dispatch is not a function

This issue can be replicated by running demo/ready-state.html after applying PR #86.

@ankon
Copy link

ankon commented Jun 20, 2017

Record-keeping: #89 probably further complicates use in the hybrid mode.

@brettpostin
Copy link

brettpostin commented Aug 17, 2017

Any update on this? We are using Polymer 2 but have elements currently in Polymer 2.x legacy mode.

As such the behavior no longer works. Is there a workaround or do I need to downgrade version?

Will 0.4.2 even work with Polymer 2?

@ronnyroeller
Copy link
Contributor Author

Here are the steps that we ended up taking:

  1. Downgrade polymer-redux
  2. Upgrade all Polymer elements that don't use polymer-redux from legacy mode to native Polymer 2
  3. Do all upgrading one can do in Polymer elements without bringing them to native element (e.g. replace this.fire(), etc)
  4. Upgrade polymer-redux + all Polymer elements using polymer-redux in one go

It's not really ideal as we still had to change dozens of elements during the last step - but at least the last step was rather mechanical.

@brettpostin
Copy link

Thanks @ronnyroeller. Simply downgrading to 0.4.2 seemed to work fine for me.

Would still like an upgrade path however.

@tur-nr
Copy link
Owner

tur-nr commented Aug 18, 2017

The logic for property setting is different between v0 and v1 in PolymerRedux. I'm not so sure if it can be done without bringing some of the old logic from v0 across to v1.

I would be happy to work with someone who knows how Hybrid elements work. From my own understanding the plan of action would be:

  • Attach dispatch()/getState() via old lifecycle events, as per v0.
  • Use fire() if element.fire === 'function' exists.

@ergo
Copy link

ergo commented Aug 23, 2017

I think the best approach maybe would be to ping polymer team? @robdodson @tjsavage

@robdodson
Copy link

@kevinpschaaf may have some ideas.

@kevinpschaaf
Copy link

kevinpschaaf commented Aug 26, 2017

So if I understand, in v1.0.0 PolymerRedux changed from being a behavior (object) to a mixin (function). There's not a straightforward way to safely auto-convert mixins back to behaviors due to the way super works in ES6 (i.e., we can provide Polymer.mixinBehaviors(), but not Polymer.makeBehaviorFromMixin()).

That said, with the current construction of the PolymerRedux mixin, it looks possible to do it manually, if that's a solution for you. I did a proof of concept here: https://glitch.com/edit/#!/brick-raver?path=public/index.html:43:2

Basically, this code adds back the behavior-based interface to the mixin function, such that it can be used as a mixin on classes OR a behavior object in hybrid elements.

  const MixinFactory = PolymerRedux;
  PolymerRedux = store => {
    let curr;
    const mixin = MixinFactory(store);
    const base = mixin(class {
      constructor() { return curr; }
      connectedCallback() {}
      disconnectedCakkback() {}
    });
    const behavior = {
      created() {
        curr = this;
        new base();
        curr = null;
      },
      attached: base.prototype.connectedCallback,
      detached: base.prototype.disconnectedCallback,
      dispatch: base.prototype.dispatch,
      getState: base.prototype.getState,
      registered() {
        this.constructor.actions = this.actions;
      }
    };
    return Object.assign(mixin, behavior);
  }
  ...
  // Can be used as a behavior OR a mixin
  const ReduxMixinOrBehavior = PolymerRedux(store);

It's a bit brittle/hacky, but something like this might work as a stop-gap solution for folks here.

Alternatively, @tur-nr could just consider factoring the code in the mixin callbacks out into shared functions and provide an option to return a behavior that uses the shared code, to avoid needing to deal with super issues. We did something like this for MutableData vs. MutableDataBehavior.

@AliMD
Copy link

AliMD commented Jan 23, 2018

@kevinpschaaf thank you very much.

@tur-nr could you please add this as a behavior.htm in the project?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants