Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP dropped composition API, adjustements to Vue3 #27

Closed
wants to merge 1 commit into from

Conversation

Valian
Copy link

@Valian Valian commented Oct 18, 2020

Related to #25

Still WIP, some tests are failing when comparing rendered templates

Also, I had to add {immediate: true} to all watch() calls in tests, because it looked like the right thing to do. Is it correct @davidmeirlevy?

@davidmeirlevy
Copy link
Contributor

You cannot change the test in order to make the new version work.
Thousands of weekly downloads in NPM use this library exactly as it is now, and it should work exactly the same for the next version of vue.

@Valian
Copy link
Author

Valian commented Oct 19, 2020

@davidmeirlevy so you would like this library to be compatible with both vue 2 and 3? I understand dropping support for vue 2 is not an option, but maybe releasing it with a separate tag, like vuex-composition-helpers-next would do?

@davidmeirlevy
Copy link
Contributor

we can make the same package but with a @next tag on the version, like: npm i [email protected].
but the behavior should be the same, so everything that worked without immediate: true should remain the same.

@Valian
Copy link
Author

Valian commented Oct 19, 2020

I can try to do that, but honestly I don't understand that behaviour. It looks like currently watch is triggered as soon as it's declared, even if there is no change. Test snippet:

                it('should trigger a watcher according a getter change', async () => {
			const watcher = jest.fn();
			const store = new Vuex.Store({
				state: {val: 'test-demo' + Math.random()},
				getters: {testGetter: (state) => state.val}
			});

			const wrapper = shallowMount({
					template: '<div>{{val}}</div>',
					setup() {
						const {testGetter} = useGetters(store, ['testGetter']);
						watch(testGetter, watcher);
						return {val: testGetter}
					}
				},
				{localVue}
			);
                         // <--- This line, it looks like it was already triggered without change
			expect(watcher).toBeCalledTimes(1); 

			store.state.val = 'new value' + Math.random();
			expect(watcher).toBeCalledTimes(1);

			// wait for rendering
			await wrapper.vm.$nextTick();

			expect(watcher).toBeCalledTimes(2);
		});

This behaviour is against the definition of watch - https://vuejs.org/v2/api/#vm-watch. Without immediate: true it shouldn't be called. Do you see the problem?

I think I'm having that issue because package.json is using a very old composition-api: 0.4.0. Version 0.6.0 fixed that. Upgrading composition api to the newest version on the master branch is probably a good idea before trying to make this package work with Vue 3.

@davidmeirlevy
Copy link
Contributor

You probably right..
Upgrading vue2 composition api to see how it works on upgraded version is mandatory too.

@davidmeirlevy
Copy link
Contributor

I already made "next" branch (and also "next" tag for the npm package) to use with vue3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants