Skip to content

Commit

Permalink
refactor: implement script setup suger for components
Browse files Browse the repository at this point in the history
  • Loading branch information
mutoe committed Feb 26, 2021
1 parent 2cfb9a7 commit c0c983d
Show file tree
Hide file tree
Showing 28 changed files with 709 additions and 1,238 deletions.
20 changes: 8 additions & 12 deletions .eslintrc
Expand Up @@ -5,24 +5,20 @@
"parser": "@typescript-eslint/parser",
"project": "./tsconfig.json",
"sourceType": "module",
"extraFileExtensions": [
".vue",
".d.ts"
]
"extraFileExtensions": [".vue", ".d.ts"]
},
"extends": [
"standard",
"plugin:@typescript-eslint/eslint-recommended",
"plugin:@typescript-eslint/recommended",
"standard-with-typescript",
"plugin:vue/vue3-recommended"
"plugin:vue/vue3-recommended",
"standard-with-typescript"
],
"rules": {
"no-undef": "off",
"comma-dangle": [
"warn",
"always-multiline"
],
"@typescript-eslint/promise-function-async": "off"
"no-unused-vars": "off",
"comma-dangle": ["warn", "always-multiline"],
"@typescript-eslint/promise-function-async": "off",
"@typescript-eslint/no-unused-vars": "off"
}
}
}
2 changes: 1 addition & 1 deletion README.md
Expand Up @@ -31,7 +31,7 @@ yarn build
- [x] Unit test ([Vue Test Utils](https://github.com/vuejs/vue-test-utils-next))
- [ ] Unit test ([Vue Testing Library](https://testing-library.com/docs/vue-testing-library/intro)) (Temporarily unavailable)
- [x] E2E test ([Cypress](https://docs.cypress.io))
- [ ] [SFC Script Setup](https://github.com/vuejs/rfcs/blob/sfc-improvements/active-rfcs/0000-sfc-script-setup.md) (Unstable)
- [x] [SFC Script Setup](https://github.com/vuejs/rfcs/blob/sfc-improvements/active-rfcs/0000-sfc-script-setup.md) (Experimental)
- [x] Vetur Tools: [VTI](https://github.com/mutoe/vue3-realworld-example-app/pull/28) and [optionally IDE hints](https://github.com/mutoe/vue3-realworld-example-app/commit/8367f89a99c467d181d9c7f4144deb05cec55210#commitcomment-43957089)

# Contributors
Expand Down
3 changes: 0 additions & 3 deletions cypress.json
@@ -1,7 +1,4 @@
{
"baseUrl": "https://mutoe.github.io/vue3-realworld-example-app",
"experimentalNetworkStubbing": true,
"chromeWebSecurity": false


}
10 changes: 5 additions & 5 deletions package.json
@@ -1,6 +1,6 @@
{
"name": "vue3-realworld-example-app",
"version": "1.0.0",
"version": "1.1.0",
"license": "MIT",
"scripts": {
"dev": "vite",
Expand All @@ -15,8 +15,8 @@
"@harlem/core": "^1.1.0",
"deepmerge": "^4.2.2",
"dompurify": "^2.2.6",
"marked": "^2.0.0",
"vue": "^3.0.2",
"marked": "^1.2.9",
"vue": "^3.0.6",
"vue-router": "^4.0.4"
},
"devDependencies": {
Expand All @@ -25,6 +25,7 @@
"@testing-library/vue": "^6.3.4",
"@types/dompurify": "^2.2.1",
"@types/jest": "^26.0.20",
"@types/marked": "^1.2.2",
"@typescript-eslint/eslint-plugin": "^4.15.2",
"@typescript-eslint/parser": "^4.15.2",
"@vitejs/plugin-vue": "^1.1.4",
Expand All @@ -39,14 +40,13 @@
"eslint-plugin-import": "^2.22.1",
"eslint-plugin-node": "^11.1.0",
"eslint-plugin-promise": "^4.3.1",
"eslint-plugin-standard": "^5.0.0",
"eslint-plugin-vue": "^7.6.0",
"husky": "^5.1.1",
"jest": "^26.6.3",
"jsdom": "^16.4.0",
"lint-staged": "^10.5.4",
"ts-jest": "^26.5.1",
"typescript": "^4.1.5",
"typescript": "~4.1.5",
"vite": "^2.0.2",
"vti": "^0.0.24",
"vue-jest": "^5.0.0-alpha.8"
Expand Down
11 changes: 1 addition & 10 deletions src/App.vue
Expand Up @@ -4,16 +4,7 @@
<AppFooter />
</template>

<script lang="ts">
import { defineComponent } from 'vue'
<script lang="ts" setup>
import AppFooter from './components/AppFooter.vue'
import AppNavigation from './components/AppNavigation.vue'
export default defineComponent({
name: 'App',
components: {
AppNavigation,
AppFooter,
},
})
</script>
8 changes: 0 additions & 8 deletions src/components/AppFooter.vue
Expand Up @@ -21,11 +21,3 @@
</div>
</footer>
</template>

<script lang="ts">
import { defineComponent } from 'vue'
export default defineComponent({
name: 'AppFooter',
})
</script>
30 changes: 10 additions & 20 deletions src/components/AppLink.vue
@@ -1,29 +1,19 @@
<template>
<router-link
:to="to"
v-bind="attrs"
>
<router-link :to="props" v-bind="attrs">
<slot />
</router-link>
</template>

<script lang="ts">
import { defineComponent, PropType } from 'vue'
import type { RouteParams } from 'vue-router'
<script lang="ts" setup>
import type { AppRouteNames } from '../router'
import type { RouteParams } from 'vue-router'
import { defineProps, useContext } from 'vue'
export default defineComponent({
name: 'AppLink',
props: {
name: { type: String as PropType<AppRouteNames>, required: true },
params: { type: Object as PropType<RouteParams>, default: () => ({}) },
},
setup (props, { attrs }) {
return {
to: props,
attrs,
}
},
})
const props = defineProps<{
name: AppRouteNames
params?: RouteParams
}>()
const { attrs } = useContext()
</script>
112 changes: 45 additions & 67 deletions src/components/AppNavigation.vue
@@ -1,42 +1,29 @@
<template>
<nav class="navbar navbar-light">
<div class="container">
<AppLink
class="navbar-brand"
name="global-feed"
>
conduit
</AppLink>
<AppLink class="navbar-brand" name="global-feed"> conduit </AppLink>

<ul class="nav navbar-nav pull-xs-right">
<li
v-for="link in navLinks"
:key="link.name"
class="nav-item"
>
<li v-for="link in navLinks" :key="link.name" class="nav-item">
<AppLink
class="nav-link"
active-class="active"
:name="link.name"
:params="link.params"
>
<i
v-if="link.icon"
:class="link.icon"
/> {{ link.title }}
<i v-if="link.icon" :class="link.icon" /> {{ link.title }}
</AppLink>
</li>
</ul>
</div>
</nav>
</template>

<script lang="ts">
import { defineComponent, computed } from 'vue'
<script lang="ts" setup>
import type { RouteParams } from 'vue-router'
import type { AppRouteNames } from '../router'
import { computed } from 'vue'
import { user } from '../store/user'
interface NavLink {
Expand All @@ -47,55 +34,46 @@ interface NavLink {
display: 'all' | 'anonym' | 'authorized'
}
export default defineComponent({
name: 'AppNavigation',
setup () {
const username = computed(() => user.value?.username)
const displayStatus = computed(() => username.value ? 'authorized' : 'anonym')
const allNavLinks = computed<NavLink[]>(() => [
{
name: 'global-feed',
title: 'Home',
display: 'all',
},
{
name: 'login',
title: 'Sign in',
display: 'anonym',
},
{
name: 'register',
title: 'Sign up',
display: 'anonym',
},
{
name: 'create-article',
title: 'New Post',
display: 'authorized',
icon: 'ion-compose',
},
{
name: 'settings',
title: 'Settings',
display: 'authorized',
icon: 'ion-gear-a',
},
{
name: 'profile',
params: { username: username.value },
title: username.value || '',
display: 'authorized',
},
])
const navLinks = computed(() => allNavLinks.value.filter(
l => l.display === displayStatus.value || l.display === 'all',
))
const username = computed(() => user.value?.username)
const displayStatus = computed(() => username.value ? 'authorized' : 'anonym')
return {
navLinks,
}
const allNavLinks = computed<NavLink[]>(() => [
{
name: 'global-feed',
title: 'Home',
display: 'all',
},
{
name: 'login',
title: 'Sign in',
display: 'anonym',
},
{
name: 'register',
title: 'Sign up',
display: 'anonym',
},
})
{
name: 'create-article',
title: 'New Post',
display: 'authorized',
icon: 'ion-compose',
},
{
name: 'settings',
title: 'Settings',
display: 'authorized',
icon: 'ion-gear-a',
},
{
name: 'profile',
params: { username: username.value },
title: username.value || '',
display: 'authorized',
},
])
const navLinks = computed(() => allNavLinks.value.filter(
l => l.display === displayStatus.value || l.display === 'all',
))
</script>
35 changes: 11 additions & 24 deletions src/components/AppPagination.vue
Expand Up @@ -13,31 +13,18 @@
</ul>
</template>

<script lang="ts">
import { defineComponent, computed, toRefs } from 'vue'
<script lang="ts" setup>
import { computed, defineEmit, defineProps, toRefs } from 'vue'
import { limit } from '../services'
export default defineComponent({
name: 'AppPagination',
props: {
page: { type: Number, required: true },
count: { type: Number, required: true },
},
emits: {
'page-change': (index: number) => typeof index === 'number',
},
setup (props, { emit }) {
const { count, page } = toRefs(props)
const pagesCount = computed(() => Math.ceil(count.value / limit))
const isActive = (index: number) => page.value === index
const onPageChange = (index: number) => emit('page-change', index)
return {
pagesCount,
isActive,
onPageChange,
}
},
})
const props = defineProps<{
page: number
count: number
}>()
const emit = defineEmit<(e: 'page-change', index: number) => void>()
const { count, page } = toRefs(props)
const pagesCount = computed(() => Math.ceil(count.value / limit))
const isActive = (index: number) => page.value === index
const onPageChange = (index: number) => emit('page-change', index)
</script>

8 comments on commit c0c983d

@levchak0910
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @mutoe
I think, it would be better to implement the setup sugar (and maybe event with ref sugar) in a separate branch or repository. And leave pure Composition API in a main branch as a main way "how to write" code in .vue files
What do you think?

@mutoe
Copy link
Owner Author

@mutoe mutoe commented on c0c983d Feb 26, 2021

Choose a reason for hiding this comment

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

@levchak0910 Yeah, what you said makes sense. Are you worried that this feature will be removed?

When we use Composition API, we will never go back to Options API, and there is no verbose boilerplate code.

So I think what we can do is to tag our previous commit.

Also, until this feature is stable, I will continue to update it to let newcomers use this pleasant feature faster :)

@mutoe
Copy link
Owner Author

@mutoe mutoe commented on c0c983d Feb 26, 2021

Choose a reason for hiding this comment

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

I'm using the Testing library for testing, this can be considered a new branch, It's a good idea.

@levchak0910
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you worried that this feature will be removed?

No, I suppose this will never happen )


This suggestion appeared because script sugar force us to write code in significantly another way. There are many changes (709+ and 1,238-) on this small code base comparatively with pure Composition API. Therefore I think to have 2 versions of codebase written with pure Composition API and suggared would be better

And to be honest, I personally don't really like the script sugar (and even more the ref sugar) and don't perceive this as an improvement


Writing tests does not affect how we write application code, so here I think it is pretty ok to have this in main branch

@levchak0910
Copy link
Collaborator

@levchak0910 levchak0910 commented on c0c983d Feb 26, 2021

Choose a reason for hiding this comment

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

My main point of view that vue gives us 2 (almost) different way, how to write code and we should show this 2 ways in 2 different branches or repos

@mutoe
Copy link
Owner Author

@mutoe mutoe commented on c0c983d Feb 26, 2021

Choose a reason for hiding this comment

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

Maybe you are right. Let me create a branch for this sugar.

If this becomes the mainstream one day in the future(I hope πŸ˜„ ), I think it can be shown as the main branch.

@mrts
Copy link

@mrts mrts commented on c0c983d Dec 9, 2021

Choose a reason for hiding this comment

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

Hi @mutoe!
<script setup> is officially stable since Vue 3.2, so I think it is safe to merge <script setup> to main now.

@mutoe
Copy link
Owner Author

@mutoe mutoe commented on c0c983d Dec 10, 2021

Choose a reason for hiding this comment

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

Hi @mutoe! <script setup> is officially stable since Vue 3.2, so I think it is safe to merge <script setup> to main now.

Thanks for your reminder. I will consider merging it to main or making main branch as default branch.

Please sign in to comment.