Skip to content

Add types to dist#203

Open
rtritto wants to merge 5 commits intovlitejs:mainfrom
rtritto:add-types
Open

Add types to dist#203
rtritto wants to merge 5 commits intovlitejs:mainfrom
rtritto:add-types

Conversation

@rtritto
Copy link
Copy Markdown
Contributor

@rtritto rtritto commented Mar 17, 2026

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

Generates .d.ts files inside the dist folder.

Fix #50

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 17, 2026

@rtritto is attempting to deploy a commit to the Vlitejs Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 17, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
vlite Ready Ready Preview Apr 13, 2026 8:45pm

@rtritto
Copy link
Copy Markdown
Contributor Author

rtritto commented Mar 17, 2026

@yoriiis Lint done in 360c3a4

@yoriiis
Copy link
Copy Markdown
Member

yoriiis commented Mar 17, 2026

@rtritto I think we should convert at least one configuration example to TS to ensure that the types are interpreted correctly

Copilot AI review requested due to automatic review settings April 13, 2026 19:21
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

@rtritto
Copy link
Copy Markdown
Contributor Author

rtritto commented Apr 13, 2026

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

@yoriiis I started the Copilot review, tell me if you need a review on other PRs

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

config/rollup.config.js:86

  • Because the config always returns both JS and d.ts builds, npm run dev (Rollup watch) will also rebuild .d.ts bundles on every change. If types only need to be generated for publish/CI, consider gating createDtsConfig behind isProduction or a dedicated env flag so watch mode stays fast and dts build errors don’t block the dev loop.
export default entries.flatMap((entry) => [createConfig(entry), createDtsConfig(entry)])


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread config/rollup.config.js
Comment on lines +69 to +71
plugins: [aliasConfig, dts()],
external: [/\.css$/, /\.svg$/]
})
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

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

The emitted .d.ts bundles will likely preserve side-effect style imports (e.g. import './vlite.css' / import './cast.css'). Since the package doesn’t currently ship a declare module '*.css' shim, TypeScript consumers can hit Cannot find module '*.css' when they load these declarations. Consider adding a *.css module declaration to the published types (or stripping/redirecting CSS imports during the dts build) so the emitted declarations don’t reference CSS files without typings.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

Comment thread config/rollup.config.js
input,
output: [
{
file: `${outputDirectory}/${outputFile.replace('.js', '.d.ts')}`,
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

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

outputFile.replace('.js', '.d.ts') will replace the first occurrence of .js anywhere in the string, not necessarily the suffix. Using a suffix-only replacement (e.g. a /.js$/-style replacement) avoids surprising paths if an entry name ever contains .js earlier in the path.

Suggested change
file: `${outputDirectory}/${outputFile.replace('.js', '.d.ts')}`,
file: `${outputDirectory}/${outputFile.replace(/\.js$/, '.d.ts')}`,

Copilot uses AI. Check for mistakes.
Comment thread package.json
@yoriiis
Copy link
Copy Markdown
Member

yoriiis commented Apr 13, 2026

@rtritto I think we should convert at least one configuration example to TS to ensure that the types are interpreted correctly

Work in progress in #215

#203 will be merged before #215

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TypeScript declaration files

3 participants