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

fix: disable spinner in production mode #195

Merged
merged 2 commits into from
May 6, 2024

Conversation

northword
Copy link
Member

@northword northword commented May 6, 2024

涉及插件:git-changelog、ThumbnailHashImages

修复在 build 结束后插件的 spinner 还在转动,build 进程没有退出的问题。ref: #191 (comment)

before:

image

after:

image

VitePress 在 build 时,为 building client + server bundles...rendering pages... 分别启动了一个 ora 实例,插件启动的 ora 实例在 building client + server bundles... 这一步结束前,相当于 build 时,同时启动了两个 spinner 。

而 ora 尚不支持同时启动两个 spinner :sindresorhus/ora#116

因此我们需要在 build 时禁用插件的 spinner。

其他替代方案:

@nekomeowww nekomeowww added pkg/git-changelog Related to @nolebase/vitepress-plugin-git-changelog pkg/thumbnail-hash Related to @nolebase/vitepress-plugin-thumbnail-hash bug Something isn't working labels May 6, 2024
Copy link

github-actions bot commented May 6, 2024

✅ Successfully deployed to Netlify

Platform Status URL
Ubuntu Success https://6638b14a4e766aaa8a38b38e--nolebase-integrations.netlify.app
Windows Success https://6638b153822f94b3c41cb495--nolebase-integrations.netlify.app

@northword
Copy link
Member Author

By the way: 推送到 netlify 时候或许可以指定一个 alias,例如 deploy-preview-${pr_num}-${platform} ,这样每次 PR 更新后重新生成的链接就是一样的了(观感上或许会好一些?区分先前的话可以在 comment 里加上 hash 或者 time)

@nekomeowww
Copy link
Member

nekomeowww commented May 6, 2024

By the way: 推送到 netlify 时候或许可以指定一个 alias,例如 deploy-preview-${pr_num}-${platform} ,这样每次 PR 更新后重新生成的链接就是一样的了(观感上或许会好一些?区分先前的话可以在 comment 里加上 hash 或者 time)

这没有什么观感上的问题吧?
如果出现缓存问题怎么办呢?
即便是有稳定的 PR 级预览,我也觉得应该有一个 commit based 预览。

Copy link
Member

@nekomeowww nekomeowww left a comment

Choose a reason for hiding this comment

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

这个修复太暴力了,相当于是直接删掉了这个功能,我觉得不太行。

@northword
Copy link
Member Author

By the way: 推送到 netlify 时候或许可以指定一个 alias,例如 deploy-preview-${pr_num}-${platform} ,这样每次 PR 更新后重新生成的链接就是一样的了(观感上或许会好一些?区分先前的话可以在 comment 里加上 hash 或者 time)

这没有什么观感上的问题吧? 如果出现缓存问题怎么办呢? 即便是有稳定的 PR 级预览,我也觉得应该有一个 commit based 预览。

应该不存在缓存问题吧,VitPress 生成的文件都是带有 hash 的,内容变化会导致 hash 变化,然后会重新加载(我也不确定)。

这个修复太暴力了,相当于是直接删掉了这个功能,我觉得不太行。

dev 下仍是存在的,

image

build 时,本就有 building client + server bundles... ,这个的含义以及包含了插件所做的事情?

@nekomeowww
Copy link
Member

dev 下仍是存在的,

事实上我个人主观看来,dev 反而是最不需要 spinner 的...

本就有 building client + server bundles... 这个的含义以及包含了插件所做的事情?

我觉得是两个事情。

building client + server bundles... 是 VitePress 自己的 pipeline 的状态。
在 CI 里面如果需要 debug 插件的话,如何能很好的看出来是运行到了哪个层级了呢?

@nekomeowww
Copy link
Member

应该不存在缓存问题吧,VitPress 生成的文件都是带有 hash 的,内容变化会导致 hash 变化,然后会重新加载(我也不确定)。

如果是 VitePress + PWA 或者纯 PWA 呢?这个时候要面对的缓存就不是很好处理了。
我觉得你可以加 PR based URL,但也要拿出 commit based URL 来方便大家按 commit 进行 debug。

@nekomeowww
Copy link
Member

nekomeowww commented May 6, 2024

有点奇怪,我理解这个问题的根源是 build 结束速度超级快,比 GitChangelog 的速度快的时候,build 会优先于 GitChangelog 结束。

  1. 首先,因为 spinner 是通过不断刷新 stdout 的行来实现的,我觉得重叠在一起不是什么问题。
  2. 然后,Git log 的获取是在 buildStart 的 hook 进行的,根据 rollup 文档所述:
image

buildStart 是并行的,是不是可以通过把 Git log 的获取放到 configResolved hook 来做来避免呢?
3. 最后的提问,为什么 VitePress 不遵循 buildEnd 钩子,等待 Vite/Rollup 结束后再提示 build complete 呢?是不是 VitePress 本身可以做出一些修改?
4. 是不是 bug?build complete 是哪一步操作来着?是输出 client bundle 吗?还是 render markdown 为 Vue 组件?因为后面还有个 render 的步骤,我觉得只要 render 步骤还在,就没什么问题吧?如果 render 也结束了,这个时候 VitePress 也退出了,我们的插件还在运行的话,Vite 或者 Rollup 可能在维护 Promise 上有点问题,或者是因为我们错误的实现了 child_process(假设)。

@nekomeowww nekomeowww added the question Further information is requested label May 6, 2024
@northword
Copy link
Member Author

我对 Vite 和 Rollup 不是很了解,事实上,这是我第一次接触和修改 Vite 插件。以下内容可能有较大的理解错误:

首先,因为 spinner 是通过不断刷新 stdout 的行来实现的,我觉得重叠在一起不是什么问题。
如果 render 也结束了,这个时候 VitePress 也退出了

重在一起不是大问题,最多不好看,现在是,windows 上,build 会持续这个状态不能退出,必须 ctrl+c:

image

但很奇怪,CI 上是正常结束的。

buildStart 是并行的

我理解的是,多个插件的 buildStart 是并行的,而不是 buildStart 与 build 是并行的,所以修改这个顺序似乎是无效的

为什么 VitePress 不遵循 buildEnd 钩子,等待 Vite/Rollup 结束后再提示 build complete 呢?是不是 VitePress 本身可以做出一些修改?

最后的提问,为什么 VitePress 不遵循 buildEnd 钩子,等待 Vite/Rollup 结束后再提示 build complete 呢?是不是 VitePress 本身可以做出一些修改?

VitePress 不是一个 Vite 插件,它在构建时

不是很清除这里面的一些深层次问题,可能理解有错误。

@nekomeowww
Copy link
Member

但很奇怪,CI 上是正常结束的。

这是 Windows 特有的 Bug 吗?我在 macOS 也无法复现这样的问题。

VitePress 不是一个 Vite 插件,它在构建时

对的,它不是 Vite 插件,它是生成 Vite dev 和 Vite build 的外部守护进程 CLI。但这只是 cli 的部分的能力,剩下的,它还是实现了很多插件和自动化能力去进行配置,你可以理解为是一系列用户构建文档页面的插件合集。

对,大概是对的,很感谢你花时间去看代码。
我想探讨的是,为什么在 buildStart 开始的 GitChangelog 会在 Vite 的 buildEnd 阶段(我们就把 hook 当作阶段吧)都结束了还在运行,这里面要么是我们自己有关 child_process 的写法有问题,要么是 ora 或者 node 自己有问题。

@northword
Copy link
Member Author

这是 Windows 特有的 Bug 吗?

看起来是的

我想探讨的是,为什么在 buildStart 开始的 GitChangelog 会在 Vite 的 buildEnd 阶段(我们就把 hook 当作阶段吧)都结束了还在运行,这里面要么是我们自己有关 child_process 的写法有问题,要么是 ora 或者 node 自己有问题。

我简单试了下,不运行任何 execa,直接返回空值,是正常退出的;
用ora和execa另外搭了一个复现,结果可以正常退出,

image

child_process 有问题的可能性更大,我看看。

@nekomeowww
Copy link
Member

要不要这样呢... 如果临时替换掉是可以修复的,我们可以先做一个 hotfix,把 ora 拿掉,就简单打印一下带颜色的 modulePrefix + 步骤,这样也不影响大家使用,也比较好实现,然后我们可以慢慢一起研究这个问题。

@northword
Copy link
Member Author

northword commented May 6, 2024

要不要这样呢... 如果临时替换掉是可以修复的,我们可以先做一个 hotfix,把 ora 拿掉,就简单打印一下带颜色的 modulePrefix + 步骤,这样也不影响大家使用,也比较好实现,然后我们可以慢慢一起研究这个问题。

我测试了一下,可以把 isSilent (禁用所有输出,包括 spinner动画 和 text)改为 isEnabled(仅禁用 spinner动画,正常打印 text),这样既可以满足需求(既打印插件信息,也不影响正常退出):

image

事实上,CI 中 window 可以正常退出可能也是在 ci 环境 isEnabled 默认是禁用的:https://github.com/sindresorhus/ora?tab=readme-ov-file#isenabled

@nekomeowww
Copy link
Member

image

能继续保留在单独的一行输出的能力吗

@northword
Copy link
Member Author

能继续保留在单独的一行输出的能力吗

image

我怀疑这仍然是 Windows 上终端输出的问题,因为 Action 的日志里是换了行的

我在 GitHub 网站上看不到最新一次 run 的 docs:build 的日志,所以只能从 raw log 看。

可以在代码里强制让 spinner 前换行,但是得到的效果也并不是很好,所以感觉不是很有必要加这个换行:

image

目前的行为只是在 build 里禁用了 spinner,理论上他应该和禁用前在 ci 里的行为是相同的。至于本地的效果,我这边没有 mac/linux 设备,不太好确认到底是不是 windows 的问题。

@nekomeowww nekomeowww merged commit d16af5a into nolebase:main May 6, 2024
8 checks passed
@northword northword deleted the fix/spinner branch May 6, 2024 13:49
@nekomeowww nekomeowww added this to the 2.0.0 milestone May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pkg/git-changelog Related to @nolebase/vitepress-plugin-git-changelog pkg/thumbnail-hash Related to @nolebase/vitepress-plugin-thumbnail-hash question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants