Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

TS-ify use-load-more and use TS and defineComponent in layouts and pages #1473

Merged
merged 3 commits into from
Jun 2, 2022
Merged

TS-ify use-load-more and use TS and defineComponent in layouts and pages #1473

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
6150d6f
TS-ify use-load-more and use TS and defineComponent in layouts and pages
obulat May 26, 2022
09cfeb9
Merge branch 'main' into add/ts_pages
obulat Jun 2, 2022
7338136
Apply suggestions from code review
obulat Jun 2, 2022
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
26 changes: 0 additions & 26 deletions src/composables/use-load-more.js
View file
Open in desktop

This file was deleted.

21 changes: 21 additions & 0 deletions src/composables/use-load-more.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { computed, ComputedRef } from '@nuxtjs/composition-api'

import { useMediaStore } from '~/stores/media'

/**
* Fetches media on 'Load More' button click.
*/
export const useLoadMore = (searchTerm: ComputedRef<string>) => {
const canLoadMore = computed(() => searchTerm.value.trim() !== '')

const onLoadMore = async () => {
const mediaStore = useMediaStore()
if (canLoadMore.value) {
await mediaStore.fetchMedia({
shouldPersistMedia: true,
})
}
}

return { canLoadMore, onLoadMore }
}
2 changes: 1 addition & 1 deletion src/composables/use-media-query.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,6 @@ export const isMinScreen = (breakpointName: Breakpoint, options?: Options) => {
/**
* Check if the user prefers reduced motion or not.
*/
export function useReducedMotion(options: Options) {
export function useReducedMotion(options?: Options) {
return useMediaQuery('(prefers-reduced-motion: reduce)', options)
}
3 changes: 1 addition & 2 deletions src/layouts/error.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@ import { defineComponent } from '@nuxtjs/composition-api'

import VFourOhFour from '~/components/VFourOhFour.vue'

const Error = defineComponent({
export default defineComponent({
name: 'ErrorPage',
components: { VFourOhFour },
layout: 'blank',
props: ['error'],
})
export default Error
</script>
2 changes: 1 addition & 1 deletion src/pages/about.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@
</VContentPage>
</template>

<script>
<script lang="ts">
Copy link
Member

Choose a reason for hiding this comment

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

Should we include this page and the others in the tsconfig.json file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great question, @krysal !

If we include this page in tsconfig, then it will be typechecked. And if the types are incorrect, it will not build the app. And because we have some dependencies on Aria-kit that uses some React types internally, we cannot typecheck many of the pages/components that have popups or modals in them.

When we only add lang="ts", but do not add the files to tsconfig, we can use Typescript in the code, but type errors will not prevent the app from building. So, we can use the TS type annotations and declare types instead of using the JSDoc comments in these components. I feel it is easier to read the code when all components use the same type annotation style, you don't have to context-switch as much.

import { defineComponent } from '@nuxtjs/composition-api'

import VLink from '~/components/VLink.vue'
Expand Down
12 changes: 9 additions & 3 deletions src/pages/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,13 @@
</template>

<script>
import { onMounted, ref, useContext, useRouter } from '@nuxtjs/composition-api'
import {
defineComponent,
onMounted,
ref,
useContext,
useRouter,
} from '@nuxtjs/composition-api'

import { ALL_MEDIA, supportedSearchTypes } from '~/constants/media'
import { isMinScreen } from '~/composables/use-media-query'
Expand All @@ -150,7 +156,7 @@ import imageInfo from '~/assets/homepage_images/image_info.json'
import OpenverseLogo from '~/assets/logo.svg?inline'
import OpenverseBrand from '~/assets/brand.svg?inline'

export default {
export default defineComponent({
name: 'HomePage',
components: {
OpenverseLogo,
Expand Down Expand Up @@ -242,7 +248,7 @@ export default {
},
],
},
}
})
</script>

<style>
Expand Down
4 changes: 2 additions & 2 deletions src/pages/search/audio.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ export default defineComponent({
focusFilters.focusFilterSidebar(event, Focus.Last)
}
}

const { canLoadMore, onLoadMore } = useLoadMore(props)
const searchTermRef = computed(() => props.searchTerm)
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW you can also use toRef for this. I think it might convey the intention a little more clearly than compute:

Suggested change
const searchTermRef = computed(() => props.searchTerm)
const searchTermRef = toRef(props, 'searchTerm')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you! I finally understood what toRef is used for! :)

const { canLoadMore, onLoadMore } = useLoadMore(searchTermRef)

return {
results,
Expand Down
4 changes: 3 additions & 1 deletion src/pages/search/image.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ export default defineComponent({
useMeta({ title: `${props.searchTerm} | Openverse` })

const results = computed(() => props.resultItems.image)
const { canLoadMore, onLoadMore } = useLoadMore(props)

const searchTermRef = computed(() => props.searchTerm)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const searchTermRef = computed(() => props.searchTerm)
const searchTermRef = toRef(props, 'searchTerm')

const { canLoadMore, onLoadMore } = useLoadMore(searchTermRef)

const focusFilters = useFocusFilters()
const handleShiftTab = () => {
Expand Down
6 changes: 4 additions & 2 deletions src/pages/search/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
</template>

<script>
import { useMeta, defineComponent } from '@nuxtjs/composition-api'
import { computed, defineComponent, useMeta } from '@nuxtjs/composition-api'

import { useLoadMore } from '~/composables/use-load-more'
import { propTypes } from '~/pages/search/search-page.types'
Expand All @@ -17,7 +17,9 @@ export default defineComponent({
setup(props) {
useMeta({ title: `${props.searchTerm} | Openverse` })

const { canLoadMore, onLoadMore } = useLoadMore(props)
const searchTermRef = computed(() => props.searchTerm)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const searchTermRef = computed(() => props.searchTerm)
const searchTermRef = toRef(props, 'searchTerm')

const { canLoadMore, onLoadMore } = useLoadMore(searchTermRef)

return { canLoadMore, onLoadMore }
},
head: {
Expand Down