Polymer / pwa-helpers

Small helper methods or mixins to help you build web apps.
BSD 3-Clause "New" or "Revised" License
440 stars 48 forks source link

A solution to make connect-mixin work again. #60

Closed busynest closed 5 years ago

busynest commented 5 years ago

connect-mixin.js : (problem) connectedCallback() { if (super.connectedCallback) { super.connectedCallback(); } this._storeUnsubscribe = store.subscribe(() => this.stateChanged(store.getState())); this.stateChanged(store.getState()); } (solution) connectedCallback() { if (super.connectedCallback) { ----> this.__storeUnsubscribe = store.subscribe(() => this._stateChanged(store.getState())); ----> this._stateChanged(store.getState()); super.connectedCallback(); } this._storeUnsubscribe = store.subscribe(() => this.stateChanged(store.getState())); this.stateChanged(store.getState()); }

I just pulled the code from a previous version and put it back in. now pwa-helpers work again for me. :)

keanulee commented 5 years ago

So in that commit I just moved the store.subscribe() and this.stateChanged() calls to after super.connectedCallback(). I wonder why this would break you... are you using LitElement?

busynest commented 5 years ago

Yes, I am using LitElement. When I start a new project by downloading npm: pwa-helpers. Nothing works. but when I use pwa-starter-kit, everything works because the package is set to 0.9.0

so with my testing, I just injected the two lines back in, then the new projects work.

I have been using old pwa-helper files for months because of this.

Oh i see that you have the code after the connectedCallback. but it seems to work within the if statement.

I think it has something to do with the super().

I am no expert, but we call constructor > super . so wouldn't all connectedCallback be super()?

busynest commented 5 years ago

I bet git is updated, but npm is not.

keanulee commented 5 years ago

pwa-starter-kit uses 0.9.0 and the latest on NPM (which is up to date with GitHub) is 0.9.1, but the relevant code hasn't changed between these two versions.

The call to super.connectedCallback() is necessary (LitElement uses it to start the initial render), but it doesn't return so the following code should execute. Curious why it doesn't work for you - if you implement connectedCallback(), does your element call super.connectedCallback()?

class MyApp extends connect(store)(LitElement) {
  connectedCallback() {
    super.connectedCallback();

    // element-specific stuff.
  }
}
busynest commented 5 years ago

you are right, everything is up to date. Usually, I do NOT have connectedCallback() written out, because I don't know how to use it.

but I re-ran all my tests. even with super.connectedCallback() text initiated. doesn't make a difference.

The only thing that works, is placing this statement back within if (super.connectedCallback()): this._storeUnsubscribe = store.subscribe(() => this.stateChanged(store.getState()));

busynest commented 5 years ago

connectedCallback() is invoked each time the custom element is appended into a document. while attributeChangedCallback is invoked when one of the custom element's attributes is added, removed, or changed.

This is why the code does NOT execute outside the if statement.

connectedCallback() is read when appended, read but not executed during change. But the if statement is allowed to execute because it bypasses somehow.

keanulee commented 5 years ago

Looking at your GitHub history, I noticed that in package.json of busynest/contractors you reference a GitHub repo instead of a version number. This won't work because the GitHub repo is in TypeScript only and doesn't contain the built JavaScript version. Consider switching this to a version or semver range, as in pacakge.json of pwa-starter-kit

"pwa-helpers": "^0.9.0",
busynest commented 5 years ago

yes, I have taken care of this typo, and it is good the typo was there, stopped me from updating lol.

The problem occurs when creating a new project. (user-account).

I have also noticed the old files are all js, as the 0.9.1 is half type script. should I be installing something else for typescript or is it just supposed to work?

busynest commented 5 years ago

https://github.com/busynest/user/blob/master/package.json

this is the new project. just the first upload, so really messy.

but you can see that I loaded it up with dependancies looking for a solution

this happens at all new projects when adding npm install --save ect.

keanulee commented 5 years ago

I also see a GitHub reference there ("pwa-helpers": "github:Polymer/pwa-helpers",) - it needs to be a version or semver range. The versioned releases contain built JavaScript files and TypeScript typings. They also contain the source TypeScript in case you want to incorporate them, but by default most people just consume the built JavaScript files (which aren't on GitHub because they're not source code).

If you remove the pwa-helpers line from package.json and run npm install pwa-helpers, it should add the right semver range for you:

"pwa-helpers": "^0.9.1",
busynest commented 5 years ago

yes, I fixed those files, just like you said, a couple day ago, its just old git.

------> but I have a better solution.

Run the two statements BEFORE if statement.

solves everything

busynest commented 5 years ago

Okay, I know this time what the difference is. _stateChanged() vs stateChanged()

busynest commented 5 years ago

stateChanged(): Redux linked vs stateChanged() without :

busynest commented 5 years ago

so I just need to update my web components to have non _ : stateChanged(state) { }

I thought _stateChanged(state) came from Redux, but it I guess not.

keanulee commented 5 years ago

Oh yeah, that changed was made in the same release. Sorry, I should've pointed that out earlier.