Added support for vuex-persist #102

bartvanvliet · 2019-03-08T22:59:28Z

Hello,

So when researching this I found this commit:
vuejs/vuex#837

It essentially allows you to preserve the state when registering a module dynamically.
Right now it just uses the default values and overrides the state. (which makes you unable to use vuex-persist for example. Might be others as well)

So now you can add preserveState to @Module as a option.

Also took the test from #47 hope you don't mind 😉 @JohnCampionJr

I believe this PR will fix/close:
#44
#46
#47
... maybe others

Thanks,
Bart

codecov · 2019-03-08T23:02:06Z

Codecov Report

Merging #102 into master will decrease coverage by 11.64%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           master     #102       +/-   ##
===========================================
- Coverage   96.32%   84.67%   -11.65%     
===========================================
  Files           9        9               
  Lines         136      124       -12     
  Branches       15       13        -2     
===========================================
- Hits          131      105       -26     
- Misses          4       16       +12     
- Partials        1        3        +2

Impacted Files	Coverage Δ
src/module/index.ts	`90.32% <100%> (-9.68%)`	⬇️
src/module/staticGenerators.ts	`57.89% <0%> (-42.11%)`	⬇️
src/vuexmodule.ts	`72.72% <0%> (-21.4%)`	⬇️
src/helpers.ts	`66.66% <0%> (-13.34%)`	⬇️
src/module/stateFactory.ts	`87.5% <0%> (-12.5%)`	⬇️
src/action.ts	`100% <0%> (ø)`	⬆️
rollup.config.js	`100% <0%> (ø)`	⬆️
src/mutationaction.ts	`85% <0%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc2e2de...e84ac7b. Read the comment docs.

* ALso fixed definition

kisonay · 2019-03-12T20:00:37Z

it would be great if we could get this support

albernhagen · 2019-04-04T21:18:34Z

Can this be merged in?

Stefan-Z-Camilleri · 2019-04-13T14:00:06Z

README.md

+...
+
+-- @Module({ dynamic: true, store: store, name: 'mm' })
++ @Module({ dynamic: true, store: store, name: 'mm', preserveState: true })


This looks like a diff of a diff... maybe a bad merge?

Nah this was intentional, this is so that the user can see which line to change. Could be removed. I thought it made it more clear.

Stefan-Z-Camilleri · 2019-04-13T14:02:14Z

package.json

@@ -62,7 +63,8 @@
    "typescript": "^3.2.2",
    "vue": "^2.6.6",
    "vuepress": "^0.14.8",
-    "vuex": "^3.1.0"
+    "vuex": "^3.1.0",
+    "vuex-persist": "^2.0.0"


The change is great, but I think tying this project to vuex-persist is an unnecessary coupling.

The change can very well live without any reference to vuex-persist and merely serve as an additional ability to set the preserveState option.

First of all, it's development only. So will only be installed once you want to develop it. Since I wrote integration tests between the two libraries. How would I go ahead and test it without the library?

@Kapulara I'm not sure, but you could install it as a peer dependency perhaps?

sneko · 2019-04-18T12:07:00Z

I'm also interesting in using the power of both tools 😃

Can't wait to see that merged and released 👍

ansien · 2019-04-24T20:21:33Z

Can this be merged in please?

ansien · 2019-04-25T16:46:04Z

@championswimmer

championswimmer · 2019-04-26T16:11:02Z

I am still apprehensive of shoving vuex-persist down the throats of everyone who uses vuex-module-decorators While I am proud of my own work on vuex-persist, many people use the alternate library vuex-persistedstate too. I would not want to drag them just because one of my libraries forces using another.

…

On Fri 26 Apr, 2019, 3:20 PM Bart van Vliet, ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In package.json <#102 (comment)> : > @@ -62,7 +63,8 @@ "typescript": "^3.2.2", "vue": "^2.6.6", "vuepress": "^0.14.8", - "vuex": "^3.1.0" + "vuex": "^3.1.0", + "vuex-persist": "^2.0.0" First of all, it's development only. So will only be installed once you want to develop it. Since I wrote integration tests between the two libraries. How would I go ahead and test it without the library? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#102 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAKD7SV3PTQGTSWI7MGOHPTPSLF5BANCNFSM4G4ZDJGA> .

sneko · 2019-04-26T16:19:49Z

@championswimmer what would be the more appropriate way? Having kind of interfaces to plugin any persisting module behind the scene?

By the way, do you currently have a project where you combine both vuex-module-decorators and vuex-persist? Just to see if possible how you deal with this combo 😄

Thanks again for your work!

ansien · 2019-04-26T18:01:17Z

@championswimmer If that's the case, then please make it possible to use dynamic modules in combination with vuex-persist. This does currently not work.

bartvanvliet · 2019-04-28T00:27:32Z

I am still apprehensive of shoving vuex-persist down the throats of everyone who uses vuex-module-decorators While I am proud of my own work on vuex-persist, many people use the alternate library vuex-persistedstate too. I would not want to drag them just because one of my libraries forces using another.
…
On Fri 26 Apr, 2019, 3:20 PM Bart van Vliet, @.***> wrote: @Kapulara commented on this pull request. ------------------------------ In package.json <#102 (comment)> : > @@ -62,7 +63,8 @@ "typescript": "^3.2.2", "vue": "^2.6.6", "vuepress": "^0.14.8", - "vuex": "^3.1.0" + "vuex": "^3.1.0", + "vuex-persist": "^2.0.0" First of all, it's development only. So will only be installed once you want to develop it. Since I wrote integration tests between the two libraries. How would I go ahead and test it without the library? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#102 (comment)>, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKD7SV3PTQGTSWI7MGOHPTPSLF5BANCNFSM4G4ZDJGA .

@championswimmer

You are right about this, however all of the wording I did in the documentation I referenced it with "e.g" so not really feeling like shoving it down everyone. Just giving an example in how this would be used.

Also I added it as development dependency so It's only required for development and wont be installed with the production flag. The only reason I installed it was because I wanted to make sure it works. If you feel like it should be removed and have this as a "preserveState" functionality just let me know. I'll change the pull request to match that instead.

However I wouldn't know how to test "preserveState".

Thanks for all of the hard work you put into this library.
Bart

sneko · 2019-06-14T05:41:57Z

I'm still wondering what would you recommend @championswimmer?

From what I understand this PR uses vuex-persistonly for test purposes. If this is gone "behavior", could this PR be merged?

Thank you,

proteus-aholt · 2019-07-10T20:42:31Z

Would love to see this PR merged, or at the very least see @Module support preserveState

bartvanvliet · 2019-07-10T20:45:35Z

Would love to see this PR merged, or at the very least see @Module support preserveState

What I could do is remove the tests (@championswimmer if that's ok)

So we can merge this finally.

championswimmer · 2019-07-11T13:54:52Z

We can keep vuex-persist in peer dependencies and merge this.

…

On Wed 10 Jul, 2019, 9:45 PM Bart van Vliet, ***@***.***> wrote: Would love to see this PR merged, or at the very least see @module support preserveState What I could do is remove the tests ***@***.*** <https://github.com/championswimmer> if that's ok) So we can merge this finally. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#102?email_source=notifications&email_token=AAKD7SV3IWVMRJ5BBVKCTRDP6ZC7BA5CNFSM4G4ZDJGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZUV6DY#issuecomment-510222095>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAKD7SRBQQJSBFMMORTJWPLP6ZC7BANCNFSM4G4ZDJGA> .

bartvanvliet · 2019-07-11T14:04:22Z

@championswimmer added it as peer dependency but now build seems to be failing since tests cannot be run. How would you like to approach?

TheYves · 2019-07-12T04:38:22Z

Sorry, but I don't get it. Why not adding vuex-persist as devDependency? This way, vuex-persist is only required for development of vuex-module-decoratos itself and not for the actual users who use the library. Users of vuex-module-decorators don't have to use vuex-persist nor do they have to install it in any way, because it's not in the dependency list.

You did an awesome job with both libraries @championswimmer , kudos for that. Let's now bring them together with this fix. :-)

bartvanvliet · 2019-07-13T22:21:24Z

Hi guys.

I've started working on a project where I required these changes. So I published the changes to a new public package which everyone else can use.

@championswimmer I will deprecate the package if you approve this merge request.

Package: @bartvanvliet/vuex-module-decorators

anorborg · 2019-08-05T14:53:16Z

@Kapulara i tried your package, but doesn't seem to be working for me FYI

const vuexLocal = new VuexPersistence<ClientState>({
  strictMode: process.env.NODE_ENV === 'development',
  storage: window.sessionStorage
})

const storeOptions: StoreOptions<ClientState> = {
  plugins: [vuexLocal.plugin],
  strict: process.env.NODE_ENV === 'development'
}

@Module({
  dynamic: true,
  namespaced: true,
  name: Namespaces.ThingModule,
  stateFactory: true,
  store: store,
  preserveState: true
})
export class ThingModule extends VuexModule {
  myThings: string[] = []
// ...
}

myThings is undefined at runtime. Not sure if its because I'm using statefactory, butt just FYI. returning to the vuex-module-decorators works (obviously not persisting though).

avxkim · 2019-08-20T16:45:42Z

@championswimmer what's the current progress?

avxkim · 2019-08-24T09:42:08Z

I've used this workaround: https://stackoverflow.com/questions/55843052/vuex-not-loading-module-decorated-with-vuex-module-decorators

Seems to work.

mrijken · 2019-08-26T20:50:09Z

@webcoderkz that's not a workaround. That answer is using a static module; not a dynamic module.

avxkim · 2019-08-27T03:15:07Z

@mrijken won't it work with dynamic modules in the future?

mrijken · 2019-08-27T17:07:38Z

@webcoderkz only when this PR is merged.

avxkim · 2019-08-27T17:09:05Z

Why @championswimmer won't merge this PR?

Signed-off-by: Arnav Gupta <[email protected]>

rystaf · 2019-11-22T19:40:05Z

I was able to preserve the state of dynamic modules with vuex-persistedstate robinvdvleuten/vuex-persistedstate#225

Tronix117 · 2019-12-01T14:52:31Z

Hey @championswimmer, when can we excpect the merge to occur ?
It's a simple additional option (one more line in the code) and the PR has been opened since march...

oakromulo · 2020-01-06T18:06:09Z

@championswimmer any chance for this to be merged this year?

dnsritch · 2020-01-09T01:35:46Z

mabe I have a solution for this,I write a vuex plugin by hijacking the store.registerModule in vuex
here is the code
maybe someone can make a optimization for this
I think it could be a better solution

//#region [rgba(0,205,20,0.1)]
const dynamicRegisterPlugin = (store: Store<IRootState>) => {

  // Override dynamic module handling in vuex
  let origRegisterModule: any
  if (store.registerModule) {
    origRegisterModule = store.registerModule.bind(store)
    // your module interfaces
    type T = IAppState | IUserState | IPermissionState | ITagsViewState | ISettingsState | IHome
    //@ts-ignore
    store.registerModule = (
      path: string,
      module: Module<T, IRootState>,
      options?: ModuleOptions
    ) => {
      // console.log(store)
      if ((store.state as Object).hasOwnProperty(path)) {
        origRegisterModule(path, module, { preserveState: true })
      } else {
        origRegisterModule(path, module, options)
      }
      // origRegisterModule(path, module, options)
    }
  }
}
//#endregion


const vuexLocal = new VuexPersistence<IRootState>({
  storage: window.sessionStorage
})

// Declare empty store first, dynamically register all modules later.
export default new Vuex.Store<IRootState>({
  plugins: [dynamicRegisterPlugin, vuexLocal.plugin]
})

championswimmer · 2020-01-11T12:15:36Z

need the travis ci to pass, before merging :(

aleksanderzubkov · 2020-01-25T23:41:05Z

Can somebody merge it please?

Signed-off-by: Arnav Gupta <[email protected]>

…ecorators into pr_102

Signed-off-by: Arnav Gupta <[email protected]>

bartvanvliet added 2 commits March 8, 2019 23:36

Added support for vuex-persist

d69894c

Switched to Vue registerModule options.

3b10892

bartvanvliet added 2 commits March 9, 2019 00:02

Removed console.log from test

7b003d6

* Added documentation for when it doesn't have initial state.

fc84d3c

* ALso fixed definition

Stefan-Z-Camilleri suggested changes Apr 13, 2019

View reviewed changes

Merge branch 'master' into master

d88e1f4

bartvanvliet force-pushed the master branch from 2ba3633 to d88e1f4 Compare July 13, 2019 22:03

bartvanvliet force-pushed the master branch from 55a1ed2 to d88e1f4 Compare July 13, 2019 22:22

avxkim mentioned this pull request Aug 20, 2019

Doesn't work with vuex-persist in dynamic mode #167

Closed

refactor tests

ddade16

Signed-off-by: Arnav Gupta <[email protected]>

championswimmer added 2 commits November 23, 2019 12:42

Merge branch 'master' into master

37db6ac

Update package.json

54cfb9f

championswimmer approved these changes Jan 11, 2020

View reviewed changes

championswimmer added 6 commits February 2, 2020 16:46

update dependencies

84bcec7

Signed-off-by: Arnav Gupta <[email protected]>

0.12.0

dc2e2de

Merge branch 'master' of gh:championswimmer/vuex-module-decorators

7bd890a

0.13.0

ccf4701

Merge commit 'refs/pull/102/head' of gh:championswimmer/vuex-module-d…

5c80361

…ecorators into pr_102

fix tests

e84ac7b

Signed-off-by: Arnav Gupta <[email protected]>

championswimmer merged commit 635e8aa into championswimmer:master Feb 2, 2020

ashahbazFV mentioned this pull request Feb 4, 2020

Dynamic module state not created with hot loading (preserveState set to true) vuejs/vuex#1675

Open

Added support for vuex-persist #102

Added support for vuex-persist #102

Conversation

bartvanvliet commented Mar 8, 2019

codecov bot commented Mar 8, 2019 • edited Loading

Codecov Report

kisonay commented Mar 12, 2019

albernhagen commented Apr 4, 2019

Stefan-Z-Camilleri Apr 13, 2019

Choose a reason for hiding this comment

bartvanvliet Apr 26, 2019

Choose a reason for hiding this comment

Stefan-Z-Camilleri Apr 13, 2019

Choose a reason for hiding this comment

bartvanvliet Apr 26, 2019

Choose a reason for hiding this comment

garyo Apr 26, 2019

Choose a reason for hiding this comment

sneko commented Apr 18, 2019 • edited Loading

ansien commented Apr 24, 2019

ansien commented Apr 25, 2019

championswimmer commented Apr 26, 2019 via email

sneko commented Apr 26, 2019

ansien commented Apr 26, 2019 • edited Loading

bartvanvliet commented Apr 28, 2019 • edited Loading

sneko commented Jun 14, 2019

proteus-aholt commented Jul 10, 2019

bartvanvliet commented Jul 10, 2019

championswimmer commented Jul 11, 2019 via email

bartvanvliet commented Jul 11, 2019

TheYves commented Jul 12, 2019

bartvanvliet commented Jul 13, 2019

anorborg commented Aug 5, 2019

avxkim commented Aug 20, 2019

avxkim commented Aug 24, 2019

mrijken commented Aug 26, 2019

avxkim commented Aug 27, 2019

mrijken commented Aug 27, 2019

avxkim commented Aug 27, 2019

rystaf commented Nov 22, 2019 • edited Loading

Tronix117 commented Dec 1, 2019

oakromulo commented Jan 6, 2020

dnsritch commented Jan 9, 2020

championswimmer commented Jan 11, 2020

aleksanderzubkov commented Jan 25, 2020

codecov bot commented Mar 8, 2019 •

edited

Loading

sneko commented Apr 18, 2019 •

edited

Loading

ansien commented Apr 26, 2019 •

edited

Loading

bartvanvliet commented Apr 28, 2019 •

edited

Loading

rystaf commented Nov 22, 2019 •

edited

Loading