vuex-electron icon indicating copy to clipboard operation
vuex-electron copied to clipboard

Added ignoredPaths option

Open MaverickMartyn opened this issue 5 years ago • 23 comments

I added an option to supply an array of state paths to not persist. This includes a unit test. It simply works by cloning the state object, and deleting any properties that should not be persisted. It then persists the modified clone instead,

I think this is my first actual pull request, so I hope it lives up to everyone's expectations. Please let me know if something isn't working.

MaverickMartyn avatar Jan 31 '19 10:01 MaverickMartyn

@MaverickMartyn I played around with the changes in my local fork and everything seems to be working great. I'd recommend @MrEmelianenko to pull this in.

I think getting rid of the blacklist and whitelist might be a good idea since this solves their use cases. Also it is very dangerous to have bugs such as #19 when working with secure applications.

Stormtv avatar Feb 04 '19 23:02 Stormtv

@Stormtv I agree that blacklist/whitelists should probably be removed, but I want o note this: This PR does NOT solve all use cases of the blacklist and whitelist. We could add an "inverted" option to cover the whitelist part, but one thing to note is that the store is still saved upon each commit, even if no "to-be-persisted"-data actually changes. For example, in an app I am working on, I use the store to store the current playback time for a video. This changes very often, and persisting the store on each commit, can cause IO issues.

Using the blacklist in combination with this, however works great. I am thinking, maybe rename these things and redefine their purposes?

Maybe with an options structure like this:

{
    // Ignores specific paths on the state object, when persisting.
    ignoredPaths: ['test.testy', 'testy2.moretesty'],
    // Blocks commits from triggering a persistence update.
    ignoredCommits: ['TESTCOMMIT', 'ANOTHERCOMMIT'],
    // Changes the behavior from blacklist-like to whitelist-like. Default to false. Maybe another name?
    invertIgnored: true,
}

EDIT: Changed my proposal, to no longer suggest throwing an error, if inverting without supplying any of the other suggested params.

EDIT 2: I now have a local test with all of this implemented, with unit tests as well. It passes both the existing and my newly added unit tests at the moment. Let me know if you want to take a look at it, and I'll update my master branch. :)

EDIT 3: Pushed the changes mentioned in edit 2.

MaverickMartyn avatar Feb 07 '19 22:02 MaverickMartyn

@MrEmelianenko @Stormtv Bump?

MaverickMartyn avatar Feb 18 '19 00:02 MaverickMartyn

@MaverickMartyn can you just return early? the else if's aren't needed and:

  1. Cause the codeclimate CI test to fail
  2. Add to cyclomatic complexity
  3. Add code smell

If you remove the else if's with just if's, it'll be nice :)

NoelDavies avatar Feb 22 '19 19:02 NoelDavies

@MaverickMartyn This is also a breaking change for some people, you'd need to keep the remaining functionality in there, and also you need to update the docs for your PR :)

NoelDavies avatar Feb 22 '19 23:02 NoelDavies

I just couldn't be asked to fix the Code Climate issue at the time. :) I'll take a look at it later, and implement your suggestions after I get home.

MaverickMartyn avatar Feb 27 '19 12:02 MaverickMartyn

@MaverickMartyn please fix all issues and I'll merge this into master. Thank you!

akodkod avatar Feb 28 '19 14:02 akodkod

@MrEmelianenko One last minor issue: The bundle size is 2 kB and the file ./dist/persisted-state.js is now 2.17 kB. I didn't want to change the bundle size limit without asking first, so should I just up the limit to 3?

MaverickMartyn avatar Mar 03 '19 03:03 MaverickMartyn

@MrEmelianenko I just need to know if I can change the maximum size limit mentioned above. Other than that, everything is done.

MaverickMartyn avatar Mar 06 '19 00:03 MaverickMartyn

I personally think changing the max size to 3kB is fine. @MaverickMartyn but it would be nice is @MrEmelianenko would sign off on it :)

Stormtv avatar Mar 06 '19 18:03 Stormtv

@Stormtv @MrEmelianenko I just accepted @Stormtv's pull request, which takes care of the bundle size limit. We should be good to go now. Sorry about the slow response time. Life happens. :)

MaverickMartyn avatar Apr 02 '19 15:04 MaverickMartyn

@MaverickMartyn @MrEmelianenko Turns out we did not need to increase the bundle size. I just made sure to minify the dist files in the PR which solved bundle size violation.

Stormtv avatar Apr 03 '19 18:04 Stormtv

@MrEmelianenko Any update on this, would greatly appreciate this being included in vuex-electron.

Stormtv avatar Apr 09 '19 13:04 Stormtv

I have a need for this too. The ignore paths saved my bacon, but I'm now relying on a specific commit of the OPs PR

NoelDavies avatar Apr 16 '19 03:04 NoelDavies

@MrEmelianenko poke

dfranssen avatar Apr 18 '19 08:04 dfranssen

@MrEmelianenko Can we get this merged please?

Stormtv avatar Apr 30 '19 00:04 Stormtv

@MrEmelianenko Are you alive <3

Stormtv avatar May 08 '19 00:05 Stormtv

@MrEmelianenko Are there any issues with this PR?

Stormtv avatar May 14 '19 15:05 Stormtv

@MrEmelianenko Just pinging you again, to make sure you see it. :)

MaverickMartyn avatar Jun 12 '19 07:06 MaverickMartyn

https://github.com/vue-electron/vuex-electron/issues/44

akodkod avatar Aug 29 '19 14:08 akodkod

ping, would love to see this merged

renaldasrep avatar Jan 19 '21 14:01 renaldasrep

hi, my option is

const getMutationList = (mutation) => {
  let arr = [];

  for (const i in mutation) {
    arr.push(i);
  }

  return arr;
};
const DeviceMutationList = getMutationList(Device.mutations);

    createPersistedState({
      invertIgnored: true,
      ignoredCommits: DeviceMutationList,
      ignoredPaths: [
        'Device.currentSelected',
        'Device.deviceList',
        'state.Device.deviceList',
        'deviceList',
        'currentSelected',
      ],
    }),

my state is

{
	"state": {
		"Device": {
			"deviceList": [],
			"currentScanDeviceName": "MockPhoneName",
			"currentReconnectDeviceName": "",
			"showCurrentSelectedDeviceOffLine": false,
			"showDeviceAddingSucceed": true,
			"showDeviceAlreadyExist": false,
			"showDeviceReconnectedFailed": false,
			"showCreateDeviceConnectionFailed": false,
			"showDeviceAdding": false,
			"accountList": []
		},
		"Keyword": {
			"keyword": "",
			"needSearch": false
		},
		"QrCode": {
			"qrCode": {
				"ip": "172.23.73.103",
				"port": 9541,
				"createTime": 1614308688995
			}
		},
		"Setting": {
			"videoQuality": "high",
			"gameMode": false,
			"socketAddress": "ws://127.0.0.1:9541",
			"arsTokenPort": 9544,
			"quality": []
		},
		"Counter": {
			"main": 52
		}
	}
}

I don't want to persite the Device info in file, is there my config option error? I see the vuex.json file have the Device info, how to i config the option, Looking forward to your answer @MrEmelianenko

liuestc avatar Feb 26 '21 03:02 liuestc

hi, my option is

const getMutationList = (mutation) => {
  let arr = [];

  for (const i in mutation) {
    arr.push(i);
  }

  return arr;
};
const DeviceMutationList = getMutationList(Device.mutations);

    createPersistedState({
      invertIgnored: true,
      ignoredCommits: DeviceMutationList,
      ignoredPaths: [
        'Device.currentSelected',
        'Device.deviceList',
        'state.Device.deviceList',
        'deviceList',
        'currentSelected',
      ],
    }),

my state is

{
	"state": {
		"Device": {
			"deviceList": [],
			"currentScanDeviceName": "MockPhoneName",
			"currentReconnectDeviceName": "",
			"showCurrentSelectedDeviceOffLine": false,
			"showDeviceAddingSucceed": true,
			"showDeviceAlreadyExist": false,
			"showDeviceReconnectedFailed": false,
			"showCreateDeviceConnectionFailed": false,
			"showDeviceAdding": false,
			"accountList": []
		},
		"Keyword": {
			"keyword": "",
			"needSearch": false
		},
		"QrCode": {
			"qrCode": {
				"ip": "172.23.73.103",
				"port": 9541,
				"createTime": 1614308688995
			}
		},
		"Setting": {
			"videoQuality": "high",
			"gameMode": false,
			"socketAddress": "ws://127.0.0.1:9541",
			"arsTokenPort": 9544,
			"quality": []
		},
		"Counter": {
			"main": 52
		}
	}
}

I don't want to persite the Device info in file, is there my config option error? I see the vuex.json file have the Device info, how to i config the option, Looking forward to your answer @MrEmelianenko

Hey there guys.

I am fairly sure this repository is no longer being maintained. I have considered taking over, but I am hard pressed for time at the moment. While I am unlikely to spend much time maintaining it, feel free to use my fork, if you want.

MaverickMartyn avatar Mar 16 '21 18:03 MaverickMartyn