Conversation
|
@rtritto is attempting to deploy a commit to the Vlitejs Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@rtritto I think we should convert at least one configuration example to TS to ensure that the types are interpreted correctly |
@yoriiis I started the Copilot review, tell me if you need a review on other PRs |
There was a problem hiding this comment.
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.tsbundles on every change. If types only need to be generated for publish/CI, consider gatingcreateDtsConfigbehindisProductionor 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.
| plugins: [aliasConfig, dts()], | ||
| external: [/\.css$/, /\.svg$/] | ||
| }) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
| input, | ||
| output: [ | ||
| { | ||
| file: `${outputDirectory}/${outputFile.replace('.js', '.d.ts')}`, |
There was a problem hiding this comment.
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.
| file: `${outputDirectory}/${outputFile.replace('.js', '.d.ts')}`, | |
| file: `${outputDirectory}/${outputFile.replace(/\.js$/, '.d.ts')}`, |
This PR contains a:
Motivation / Use-Case
Generates
.d.tsfiles inside thedistfolder.Fix #50