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

fix(schema): sync types of vite v3.x #7104

Merged
merged 2 commits into from
Aug 31, 2022
Merged

fix(schema): sync types of vite v3.x #7104

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bd5ec99
feat(schema): sync types of vite v3.x
brownsugar Aug 31, 2022
4a1aa26
fix: remove `.url` declaration
danielroe Aug 31, 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
9 changes: 5 additions & 4 deletions packages/schema/src/types/global/vite.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,16 @@ export interface ViteHot {

export interface ViteGlobOptions {
as?: string
eager?: boolean
import?: string
query?: string | Record<string, string | number | boolean>
exhaustive?: boolean
}

export interface ViteImportMeta {
/** Vite client HMR API - see https://vitejs.dev/guide/api-hmr.html */
readonly hot?: ViteHot

/** vite glob import utility - https://vitejs.dev/guide/features.html#glob-import */
glob?(pattern: string, options?: ViteGlobOptions): Record<string, () => Promise<Record<string, any>>>

/** vite glob import utility - https://vitejs.dev/guide/features.html#glob-import */
globEager?(pattern: string, options?: ViteGlobOptions): Record<string, Record<string, any>>
glob (glob: string | string[], options?: ViteGlobOptions): Record<string, () => Promise<Record<string, any>>>
Copy link
Member

Choose a reason for hiding this comment

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

Tricky. This is meant to be optional as the declaration is also present in webpack builder.

It might be worth reconsidering whether we add these types within vite-builder rather than schema, so it can be more precisely targeted to both the vite version and the builder that is being used. wdyt @pi0?

Copy link
Member

Choose a reason for hiding this comment

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

I like the idea of moving types but that also makes a tricky question of how we tie vite versions to Nuxt releases (it uses caret). As you also mentioned, reason to have types here was that they are shared between different bundlers. I think we can revise this refactor later but you are right about being optional.

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
glob (glob: string | string[], options?: ViteGlobOptions): Record<string, () => Promise<Record<string, any>>>
glob?(glob: string | string[], options?: ViteGlobOptions): Record<string, () => Promise<Record<string, any>>>

Copy link
Member

@danielroe danielroe Aug 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@brownsugar seemed to say that optional chaining would break the usage.

Copy link
Member

@pi0 pi0 Aug 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

It really seems a bug in vite to me with static replacement (import.meta by spec is an object while static replacement is not covering another valid syntax of optional chaining...)

Okay then let's keep it non optional until issue is fixed by either a refactor or upstream. Webpack is not that important for being optional (we might even polyfill import.meta.glob for webpack)

}