pinia icon indicating copy to clipboard operation
pinia copied to clipboard

WritableComputed doesn't find getters for Vue2 testing

Open heykc opened this issue 2 years ago โ€ข 8 comments

Reproduction

https://github.com/heykc/pinia-test

Steps to reproduce the bug

  1. pull down the repo above
  2. run the test
  3. notice test fails with a warning of [Vue warn]: Write operation failed: computed value is readonly.

Expected behavior

The test should successfully reassign the double getter to a new value, and the expectation following should pass.

This test is nearly identical to the test inside of @pinia/testing called allows overriding computed properties but runs with a Vue 2 instance instead.

Actual behavior

The test fails with a warning of [Vue warn]: Write operation failed: computed value is readonly.

Additional information

I believe this is due to the fact that the WritableComputed function in @pinia/testing/testing.ts only looks for refs when checking for computed properties. In Vue 2, there are no refs and therefore none of the getters within a given store are spotted within this function.

I have a proposed solution which is to check for Vue2 specifically and confirm there are getters within the options of the store. The code below is an example of my solution:

function WritableComputed({ store, options }: PiniaPluginContext) {
  const rawStore = toRaw(store)
  for (const key in rawStore) {
    const value = rawStore[key]
    // Vue 2 pinia getters are not refs and have to be found a different way than isComputed()
    const isVue2Getter = isVue2 && options.getters?.[key]
    if (isComputed(value) || isVue2Getter) {
      rawStore[key] = customRef((track, trigger) => {
        let internalValue: any
        return {
          get: () => {
            track()
            // since Vue2 getters are just functions and not refs, they don't have a value property
            return internalValue !== undefined ? internalValue : (isVue2Getter ? value : value.value)
          },
          set: (newValue) => {
            internalValue = newValue
            trigger()
          },
        }
      })
    }
  }
}

I'd be happy to put in a pull request but I couldn't figure out a way to reproduce this test directly in the pinia/testing repo. It would have required a way to switch vue-demi to vue2 which caused a lot of peer dependency conflicts. I'd love to find out the best way to write a test for this particular instance to go along with this code fix.

I'm also not married to the isVue2Getter variable name or the nested ternary. Open to code suggestions on this.

heykc avatar Apr 04 '22 20:04 heykc

Currently, is there a way to mock getters in Vue 2?

kingyue737 avatar Jun 10 '22 03:06 kingyue737

Since Vue2.7, any news on this matter ?

frsimond avatar Sep 07 '22 08:09 frsimond

Is there a workaround?

stephanos-square avatar Apr 03 '23 21:04 stephanos-square

@stephanos-square you need to set the state instead, in order to make the getter generate the value you want

dennybiasiolli avatar Apr 04 '23 05:04 dennybiasiolli

@stephanos-square you need to set the state instead, in order to make the getter generate the value you want

Didn't work for me, the state was changed, but the related getters didn't react to that change

vate avatar Apr 20 '23 09:04 vate

@stephanos-square you need to set the state instead, in order to make the getter generate the value you want

Didn't work for me, the state was changed, but the related getters didn't react to that change

Do you have an example repo? It could be something related to the lifecycle of your test case. If you could share the code I can check it

dennybiasiolli avatar Apr 20 '23 10:04 dennybiasiolli

@stephanos-square you need to set the state instead, in order to make the getter generate the value you want

Didn't work for me, the state was changed, but the related getters didn't react to that change

Do you have an example repo? It could be something related to the lifecycle of your test case. If you could share the code I can check it

Here is an abridged version of what I have:

I have a factory:

const factory = () => {
	return mount(myComponent, {
		pinia: createTestingPinia(),
		localVue
	});
};

Then in the tests:

it("Shows the footer button as disabled when loading", async () => {
		wrapper = factory({});
		const loadingStore = useLoadingStore();
		loadingStore._loadingStack = ["loadingContext"];
		console.log(loadingStore._loadingStack , loadingStore.isLoading ) // logs "["loadingContext"], false"
		const footer = wrapper.find('[data-testid="footer"]');
		expect(footer.findComponent(ButtonComponent).classes("disabled")).toBe(true);
});

The console log I inserted yields the _loadingStack state which I can effectively modify, but the resulting computed isLoading checks for the length > 0 of the _loadingStack array, but, as you see, it still returns false. I have tried inserting some nexTicks in the process, but with no results.

vate avatar Apr 20 '23 10:04 vate

What about the code in your component and in the loadingStore? Maybe there's something strange there. Can you share the repo or a (not)working example?

dennybiasiolli avatar Apr 20 '23 11:04 dennybiasiolli