vuejs / pinia

🍍 Intuitive, type safe, light and flexible Store for Vue using the composition api with DevTools support
https://pinia.vuejs.org
MIT License
13.24k stars 1.07k forks source link

WritableComputed doesn't find getters for Vue2 testing #1200

Open heykc opened 2 years ago

heykc commented 2 years ago

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.

kingyue737 commented 2 years ago

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

frsimond commented 2 years ago

Since Vue2.7, any news on this matter ?

stephanos-square commented 1 year ago

Is there a workaround?

dennybiasiolli commented 1 year ago

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

vate commented 1 year ago

@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

dennybiasiolli commented 1 year ago

@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

vate commented 1 year ago

@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.

dennybiasiolli commented 1 year ago

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?

djodjonx commented 2 months ago

This is a work around for Vue2 with setup store (not tested with option) working like official documentation. You just have to create a plugin with the following code

import {
  customRef,
  isRef,
  toRaw,
} from 'vue'
import type { ComputedRef, WritableComputedRef } from 'vue'
import {
  PiniaPluginContext,
} from 'pinia'

function isComputed<T>(
  v: ComputedRef<T> | WritableComputedRef<T> | unknown,
): v is ComputedRef<T> | WritableComputedRef<T> {
  return !!v && isRef(v) && 'effect' in v
}

export function WritableComputedPlugin({ store }: PiniaPluginContext) {
  const rawStore = toRaw(store)

  for (const key in rawStore) {
   /**
     * only value in _hmrPayload are isComputed true
     */
    if (rawStore._hmrPayload.getters[key] && isComputed(rawStore._hmrPayload.getters[key])) {
      const originalComputed = Object.assign({}, rawStore._hmrPayload.getters[key])
      const originalFn = originalComputed.effect.getter
      const defaultValue = originalComputed.value
      originalComputed._value = originalComputed.value
      rawStore[key] = customRef((track, trigger) => {
        // override the computed with a new one
        const overriddenFn = () => originalComputed._value

        return {
          get: () => {
            track()
            return originalComputed._value
          },
          set: (newValue) => {
            // reset the computed to its original value by setting it to its initial state
            if (newValue === undefined) {
              originalComputed.effect.fn = originalFn
              originalComputed._value = defaultValue
              originalComputed._dirty = true
            } else {
              originalComputed.effect.fn = overriddenFn
              originalComputed._value = newValue
            }
            // this allows to trigger the original computed in setup stores
            trigger()
          },
        }
      })
    }
  }
}

then add the plugin to your renderer

pinia: createTestingPinia({
        plugins: [WritableComputedPlugin],
      }),

You can now stub your getters result then reset it by passing undefined value.