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.

Sign up for GitHub

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

Jump to bottom

chore(dep/vue) bump peerDep composition-api to 0.6 #1253

Merged
merged 3 commits into from
Jun 20, 2020
Merged

chore(dep/vue) bump peerDep composition-api to 0.6 #1253

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
65c1324
chore(dep/vue) bump peerDep composition-api to 0.6
darrenjennings Jun 20, 2020
b363a67
Update .changeset/tiny-guests-wash.md
Jun 20, 2020
7560f68
fixup! chore(dep/vue) bump peerDep composition-api to 0.6
darrenjennings Jun 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .changeset/tiny-guests-wash.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
'@xstate/vue': minor
---

chore(dep/vue) bump peerDep of @vue/composition-api to 0.6.x.

- breaking changes in 0.6.x, adapated code. Will make it easier for
this library to also be updated to Vue 3.
- Uses new watcher option since it was changed to be lazy by default
- now using shallow ref since xstate objects should not be refs deeply.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefixes like chore(dep/vue) are not needed, the changelog entry created later because of this changeset file will only appear in the ./packages/xstate-vue/CHANGELOG.md so no need to include this information here. A changeset should also only describe user-facing changes, no need to mention that, for example, a new watcher option has been used internally - it's just an implementation detail.

I propose to change this to something like this:

Suggested change
chore(dep/vue) bump peerDep of @vue/composition-api to 0.6.x.
- breaking changes in 0.6.x, adapated code. Will make it easier for
this library to also be updated to Vue 3.
- Uses new watcher option since it was changed to be lazy by default
- now using shallow ref since xstate objects should not be refs deeply.
Upgraded package for compatibility with the newest `@vue/composition-api@^0.6.0`, which means that a peer dependency requirement has changed to this version, which is a **breaking change**. The only observable behavior change is that exposed refs are now **shallow**.

Which assumes that previously exposed refs were deep, which I don't know if it was true nor what does it actually mean in context of Vue. Would love to learn more if you have a spare moment to explain this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Migrating to 0.6 seems to have changed how ref returns were typed. So actually I think it could work with traditional ref's if the type error could be resolved:

Type 'State<TContext, TEvent, TState>' is not assignable to type '{ value: string; context: UnwrapRef<TContext>; actions: ActionObject<TContext, TEvent>[]; changed?: boolean | undefined; matches: <TSV extends TState["value"]>(value: TSV) => this is TState extends { ...; } ? TState : never; }'.
  Types of property 'context' are incompatible.
    Type 'TContext' is not assignable to type 'UnwrapRef<TContext>'.

^^not sure how to resolve the UnwrapRef<TContext> typing.

A traditional ref will allow for an object's properties and sub-properties to be reactive. The "gotcha" with a shallowRef is that members are not reactive. shallowRef's continue to allow @xstate/vue to work since all ref assignments used are wholesale replaced e.g. current.value = or state.value = (vs. state.value.value =). However, this is a breaking change if users were wanting to mutate return ref's properties directly and still have reactivity like today, e.g. if state.value.value is assigned directly instead of via send, it would not trigger any vue reactivity (watchers, computed, template updates etc.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A traditional ref will allow for an object's properties and sub-properties to be reactive.

Interesting, is it Proxy-based?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the composition-api is not proxy based, though vue-next (v3) is

4 changes: 2 additions & 2 deletions packages/xstate-vue/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
"url": "https://github.com/davidkpiano/xstate/issues"
},
"peerDependencies": {
"@vue/composition-api": "^0.3.4",
"@vue/composition-api": "^0.6.5",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we depend on some features in 0.6.5 specifically or could we change this to ^0.6.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can change it. there were bugs caught in 0.6.0->.4 within a few days so didn't want to point to versions that were knowingly buggy but I suppose that's for the consumer to figure out? Was curious why it wasn't ^0.3.0

"@xstate/fsm": "^1.0.0",
"vue": "^2.6.10",
"xstate": "^4.7.8"
Expand All @@ -57,7 +57,7 @@
"devDependencies": {
"@testing-library/jest-dom": "^4.2.4",
"@testing-library/vue": "^4.1.0",
"@vue/composition-api": "^0.3.4",
"@vue/composition-api": "~0.6.5",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason why this is using ~ over ^?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, changing back. I was initially going to try and provide backward compat, but not anymore

"@vue/test-utils": "^1.0.0-beta.30",
"@xstate/fsm": "*",
"babel-core": "^6.26.3",
Expand Down
32 changes: 19 additions & 13 deletions packages/xstate-vue/src/fsm.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {
ref,
shallowRef,
isRef,
watch,
onMounted,
Expand Down Expand Up @@ -50,7 +50,7 @@ export function useMachine<
)
).start();

const state = ref<StateMachine.State<TContext, TEvent, any>>(
const state = shallowRef<StateMachine.State<TContext, TEvent, any>>(
getServiceValue(service)
);

Expand Down Expand Up @@ -80,21 +80,27 @@ export function useService<
service
)
? service
: ref(service);
const state = ref<StateMachine.State<TContext, TEvent, TState>>(
: shallowRef(service);
const state = shallowRef<StateMachine.State<TContext, TEvent, TState>>(
serviceRef.value.state
);

watch(serviceRef, (service, _, onCleanup) => {
state.value = getServiceValue(service);
watch(
serviceRef,
(service, _, onCleanup) => {
state.value = getServiceValue(service);

const { unsubscribe } = service.subscribe((currentState) => {
if (currentState.changed) {
state.value = currentState;
}
});
onCleanup(unsubscribe);
});
const { unsubscribe } = service.subscribe((currentState) => {
if (currentState.changed) {
state.value = currentState;
}
});
onCleanup(unsubscribe);
},
{
immediate: true
}
);

const send = (event: TEvent | TEvent['type']) => serviceRef.value.send(event);

Expand Down
32 changes: 19 additions & 13 deletions packages/xstate-vue/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {
ref,
shallowRef,
watch,
isRef,
onMounted,
Expand Down Expand Up @@ -68,7 +68,7 @@ export function useMachine<TContext, TEvent extends EventObject>(
rehydratedState ? State.create(rehydratedState) : undefined
);

const state = ref<State<TContext, TEvent>>(service.state);
const state = shallowRef<State<TContext, TEvent>>(service.state);

onMounted(() => {
service.onTransition((currentState) => {
Expand Down Expand Up @@ -98,18 +98,24 @@ export function useService<TContext, TEvent extends EventObject>(
} {
const serviceRef = isRef(service)
? service
: ref<Interpreter<TContext, any, TEvent>>(service);
const state = ref<State<TContext, TEvent>>(serviceRef.value.state);
: shallowRef<Interpreter<TContext, any, TEvent>>(service);
const state = shallowRef<State<TContext, TEvent>>(serviceRef.value.state);

watch(serviceRef, (service, _, onCleanup) => {
state.value = service.state;
const { unsubscribe } = service.subscribe((currentState) => {
if (currentState.changed) {
state.value = currentState;
}
});
onCleanup(() => unsubscribe());
});
watch(
serviceRef,
(service, _, onCleanup) => {
state.value = service.state;
const { unsubscribe } = service.subscribe((currentState) => {
if (currentState.changed) {
state.value = currentState;
}
});
onCleanup(() => unsubscribe());
},
{
immediate: true
}
);

const send = (event: TEvent | TEvent['type']) => serviceRef.value.send(event);

Expand Down
9 changes: 7 additions & 2 deletions packages/xstate-vue/test/UseMachine.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
</template>

<script lang="ts">
import { PropType } from 'vue';
import { useMachine } from '../src';
import { Machine, assign, Interpreter, spawn, doneInvoke, State } from 'xstate';
import { watch } from '@vue/composition-api';
Expand Down Expand Up @@ -45,8 +46,12 @@ const fetchMachine = Machine<typeof context, any>({
});

export default {
props: ['persistedState'],
setup({ persistedState }): { persistedState: State<any, any> } {
props: {
persistedState: {
type: Object as PropType<State<any>>
}
},
setup({ persistedState }) {
const onFetch = () =>
new Promise((res) => setTimeout(() => res('some data'), 50));

Expand Down
15 changes: 10 additions & 5 deletions packages/xstate-vue/test/UseService.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
</template>

<script lang="ts">
import Vue, { PropType } from 'vue'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, I don't see Vue being used here

import { useService } from '../src';
import {
Machine,
Expand All @@ -14,20 +15,24 @@ import {
spawn,
doneInvoke,
State,
Service
} from 'xstate';
import { watch, ref } from '@vue/composition-api';
import { watchEffect, ref, Ref } from '@vue/composition-api';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

both ref and Ref are unused in this file, could you remove them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea found these test .vue files are hard to type/lint. Will clean them up a bit.


export default {
props: ['service'],
setup(props): { service: Service } {
props: {
service: {
type: Object as PropType<Interpreter<any>>,
}
},
setup(props) {
let { state, send } = useService(props.service);

watch(() => {
watchEffect(() => {
let currentState = useService(props.service);
state.value = currentState.state.value;
send = currentState.send;
});

return { state, send };
}
};
Expand Down
15 changes: 10 additions & 5 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1991,12 +1991,12 @@
source-map "~0.6.1"
vue-template-es2015-compiler "^1.9.0"

"@vue/composition-api@^0.3.4":
version "0.3.4"
resolved "https://registry.yarnpkg.com/@vue/composition-api/-/composition-api-0.3.4.tgz#43d2c3377173cfe1d02e51e3342bcf0fbde9c4b6"
integrity sha512-aMbg/pEk0PSQAIFyWWLqbAmaCCTx1kFq+49KZslIBJH9Wqos7eEPLtMv4gGxd/EcciBIgfbtUXaXGg/O3mheRA==
"@vue/composition-api@~0.6.5":
version "0.6.5"
resolved "https://registry.yarnpkg.com/@vue/composition-api/-/composition-api-0.6.5.tgz#890b4ab3777ee95c6d633872db2bf53a45446d5b"
integrity sha512-IRdtn6/59yPNmvzlu5JTAR3SK0kK5T69wtz8oefSY8FFUPoOe7A2BlcVCypRX7jTmpBADhrHkAiklc8ffmPOuQ==
dependencies:
tslib "^1.9.3"
tslib "^2.0.0"

"@vue/test-utils@^1.0.0-beta.29", "@vue/test-utils@^1.0.0-beta.30", "@vue/test-utils@^1.0.0-beta.31":
version "1.0.0-beta.32"
Expand Down Expand Up @@ -13746,6 +13746,11 @@ tslib@^1.10.0, tslib@^1.8.0, tslib@^1.8.1, tslib@^1.9.0, tslib@^1.9.3:
resolved "https://registry.yarnpkg.com/tslib/-/tslib-1.11.1.tgz#eb15d128827fbee2841549e171f45ed338ac7e35"
integrity sha512-aZW88SY8kQbU7gpV19lN24LtXh/yD4ZZg6qieAJDDg+YBsJcSmLGK9QpnUjAKVG/xefmvJGd1WUmfpT/g6AJGA==

tslib@^2.0.0:
version "2.0.0"
resolved "https://registry.yarnpkg.com/tslib/-/tslib-2.0.0.tgz#18d13fc2dce04051e20f074cc8387fd8089ce4f3"
integrity sha512-lTqkx847PI7xEDYJntxZH89L2/aXInsyF2luSafe/+0fHOMjlBNXdH6th7f70qxLDhul7KZK0zC8V5ZIyHl0/g==

tslint@^5.11.0:
version "5.20.1"
resolved "https://registry.yarnpkg.com/tslint/-/tslint-5.20.1.tgz#e401e8aeda0152bc44dd07e614034f3f80c67b7d"
Expand Down