feat: Add Vite 7 support #800
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
|
❌ Deploy Preview for histoire-examples-svelte3 failed.
|
❌ Deploy Preview for histoire-examples-vue3 failed.
|
❌ Deploy Preview for histoire-site failed.
|
❌ Deploy Preview for histoire-controls failed.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @hugoattal, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request primarily focuses on a significant dependency upgrade, moving the project to Vite 7 and updating vite-node. This update aims to improve overall performance, leverage new features from these tools, and maintain compatibility with the latest ecosystem standards. I've also addressed any immediate type-related issues that arose from these upgrades.
Highlights
- Dependency Upgrades: I've upgraded the core
Vitedependency across the entire monorepo, including all examples and internal packages, from version6.xto7.0.0. This brings the project up to date with the latest Vite features and performance improvements. - Vite-Node Update: The
vite-nodedependency has been updated from0.34.7to3.2.4in the mainhistoirepackage, ensuring compatibility and leveraging improvements in the test runner and development environment. - Type Compatibility Fixes: I've introduced minor type fixes in
packages/histoire/src/node/build.tsandpackages/histoire/src/node/vite.tsto address breaking changes or stricter type checking introduced by the Vite 7 upgrade, ensuring the codebase remains type-safe. - Lockfile Synchronization: The
pnpm-lock.yamlfile has been comprehensively updated to reflect all new dependency versions, including transitive updates forrollup,esbuild,postcss,es-module-lexer,debug, andnanoid, ensuring a consistent and reproducible build environment.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
histoire
@histoire/app
@histoire/controls
@histoire/plugin-nuxt
@histoire/plugin-percy
@histoire/plugin-screenshot
@histoire/plugin-svelte
@histoire/plugin-vue
@histoire/shared
@histoire/vendors
commit: |
There was a problem hiding this comment.
Code Review
This pull request updates the project to support Vite 7 by bumping the vite dependency across all packages and also updating vite-node. The changes in package.json and pnpm-lock.yaml files are consistent with this goal.
I've identified a couple of areas in the TypeScript code where type-related workarounds have been introduced, likely to resolve issues from the dependency upgrades. I've left specific suggestions to improve the type safety and maintainability of these changes. Overall, the changes look good and align with the PR's objective.
| @@ -133,6 +133,7 @@ export async function build(ctx: Context) { | |||
| }, | |||
| }) | |||
| } | |||
| // @ts-expect-error vue plugin use function form | |||
There was a problem hiding this comment.
While using @ts-expect-error is acceptable for handling type issues from external libraries, the accompanying comment vue plugin use function form is a bit cryptic. For better long-term maintainability, it would be helpful to add a more descriptive comment explaining why this is necessary.
For example:
// HACK: Manually trigger the vue plugin's configureServer hook.
// This is needed because we create a new Vite server instance for the build,
// and the Vue plugin needs to be configured with this new instance.
// @ts-expect-error The 'configureServer' hook can be an object, but we assume it's a function here.This provides more context for future developers who might encounter this code.
| (this.meta as any).histoire = { | ||
| isCollecting: isServer, | ||
| } |
There was a problem hiding this comment.
Using as any to bypass type checking should be avoided as it can hide potential bugs and makes the code harder to maintain. It seems the goal is to add a custom histoire property to the plugin context's meta object.
A better, type-safe approach would be to use declaration merging to augment the PluginContextMeta interface from Rollup. You can add a histoire.d.ts file in your project (e.g., in the src directory) with the following content:
// histoire.d.ts
import 'rollup'
declare module 'rollup' {
export interface PluginContextMeta {
histoire?: {
isCollecting: boolean
}
}
}Make sure this file is included in your tsconfig.json. This will make the histoire property available on this.meta in a type-safe way, and you can revert this change.
As a less invasive alternative, using bracket notation is safer than as any and would also resolve the type error:
| (this.meta as any).histoire = { | |
| isCollecting: isServer, | |
| } | |
| this.meta['histoire'] = { | |
| isCollecting: isServer, | |
| } |
|
I have no idea how to fix this error with vite-node: I've been trying to fix it a long time ago, the plugin is correctly loaded, and |
|
I have commented out those two imports that seems to break everything: I once again spent hours on this with no lead 😅... This seems to be a vite-node update, but I have no clue how to fix it. Except maybe hard-code the location of the css file. In the console.log(await runner.resolveUrl("histoire-style")); //This does return the correct URLSo I don't understand why on earth why this fail: With Even with hard-coding the style file path, I get this: I think vite-node needs to be tweaked, but since there's no documentation, I can't figure it out... |
Fix #798, #792
Description
Update to Vite 7 and update vite-node.