Skip to content

Commit

Permalink
fix(SSR): explicitly load KtNavbar as a component
Browse files Browse the repository at this point in the history
A change in the template of NavBar.vue produced a very hard to trace down bug
which caused the navbar to not render in the documentation. Although the source
of this bug could not be made out, it seemed to have to do with how nuxt is
resolving components. Adding an explicit import of KtNavbar in the file (and
therefore not relying on the kotti-ui plugin for nuxt) fixes this.

As the file change that triggered this is very unrelated and it would be impossible
to see this very unrelated effect in code review, a test was added that should at
least stop the very same event to occur

Co-Authored-By: Florian Wendelborn <[email protected]>
  • Loading branch information
Isokaeder and FlorianWendelborn committed Jun 5, 2024
1 parent c4fa4a9 commit bec6660
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 1 deletion.
4 changes: 4 additions & 0 deletions packages/documentation/components/NavBar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

<script lang="ts">
import type { Kotti } from '@3yourmind/kotti-ui'
import { KtNavbar } from '@3yourmind/kotti-ui'
import { computed, defineComponent, ref } from 'vue'
import navLogo from '../assets/img/nav_logo.svg'
Expand All @@ -38,6 +39,9 @@ const saveSavedFieldsToLocalStorage = (isNarrow: boolean) => {
export default defineComponent({
name: 'NavBar',
components: {
KtNavbar,
},
setup() {
const route = useRoute()
Expand Down
2 changes: 1 addition & 1 deletion packages/documentation/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
},
"private": true,
"scripts": {
"build": "nuxt generate",
"build": "nuxt generate && ./scripts/test-for-ssr-bug.sh",
"check:eslint": "eslint --max-warnings=0 .",
"check:prettier": "yarn --cwd ../.. run prettier --check --ignore-path=.gitignore ./packages/documentation",
"check:stylelint": "stylelint './**/*.{css,scss,vue}'",
Expand Down
32 changes: 32 additions & 0 deletions packages/documentation/scripts/test-for-ssr-bug.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#!/usr/bin/env bash

set -euo pipefail

# What happened to make this test necessary?
#
# Beginning of June 2024 the documentation page had a peculiar bug: The main Navbar did not render.
#
# What was the cause of that bug?
#
# The bug was traced down to a change (that seemed entirely unrelated) in documentation/components/NavBar.vue
# Closer investigation yielded that this change had an unintended effect on how nuxt is resolving imports
# when it is rendering/hydrating components.
#
# How was this solved
#
# The fix was to make nuxt explictly resolve the component during SSR. This causes the following change to
# index.html that is generated by `nuxt generate`:
#
# - <ktnavbar class="kt-navbar">...</ktnavbar> <!-- Will be hydrated client side -->
#
# + <nav class="kt-navbar">...</nav> <!-- pre-rendered server-side -->
#
# Why this test?
#
# Since this was impossible to foresee and would likely slip through a code review in a possible future scenario
# this is a very basic sanity test to at least catch the very same scenario.

if grep '</ktnavbar>' ./dist/index.html;
then echo "The output of nuxt contains a 'ktnavbar' tag, this might be the result of a strange SSR bug" && false;
else true;
fi
1 change: 1 addition & 0 deletions packages/kotti-ui/source/kotti-navbar/KtNavbar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ export default defineComponent({
watch(
() => props.theme,
(newTheme) => {
if (typeof document === 'undefined') return // SSR
const rootElement = document.querySelector(':root') as HTMLElement
rootElement.style.setProperty(
Expand Down

0 comments on commit bec6660

Please sign in to comment.