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

refactor(types): use dts-buddy to generate types with map #751

Merged
merged 17 commits into from
Oct 26, 2023

Conversation

dominikg
Copy link
Member

and cleanup type definitions and tsconfig

This introduces a types:generate script that has to be called every time you change a type to update types/index.d.ts which is commited.

Having the generated file in the repo ensures that we are able to review changes in public types and can prevent unintended drift or breaking changes

@dominikg
Copy link
Member Author

@Rich-Harris in #658 you did an export of index.js from a d.ts files which I didn't use here. Was that something that has changed in dts-buddy or is this one here holding it wrong?

@dominikg
Copy link
Member Author

example of the script that validates types are in sync https://github.com/sveltejs/vite-plugin-svelte/actions/runs/6207076288/job/16851955290?pr=751#step:12:18

@dominikg
Copy link
Member Author

hmm, dts-buddy pulling in an older version of ts isn't optimal...

@dominikg dominikg added this to the vite-plugin-svelte 3.0 milestone Sep 16, 2023
@Rich-Harris
Copy link
Member

Bumped dts-buddy, so that it now consumes TypeScript as a peer dependency.

in #658 you did an export of index.js from a d.ts files which I didn't use here

The reason for that is that people should be able to do this...

import type { CompileOptions } from '@sveltejs/vite-plugin-svelte';

...which isn't possible if the index.js is the entry module, since it doesn't (can't) re-export the stuff in public.d.ts. In cases where we want to expose both types and values, but are authoring in .js with JSDoc, we need to treat the .d.ts file as the entry point and re-export the values from the real entry point.

@dominikg
Copy link
Member Author

hmm, i think this works. Not for CompileOptions as that is now imported from svelte-4 and only part of the Options type of vite-plugin-svelte, but here is the type in the namespace
https://github.com/sveltejs/vite-plugin-svelte/pull/751/files#diff-3ce3450c71e77215bb00652cd1afec4105620695f279d2c142552a181c3fb877R7

and at least in my local tests import type {Options} from '@sveltejs/vite-plugin-svelte' works. (for the rare case a user needs to do it at all, usually they just do

import {svelte} from '@sveltejs/vite-plugin-svelte';
import {defineConfig} from 'vite';
export default defineConfig({
  plugins: [
    svelte({
      // yay for autocomplete
    })
  ]
})

note that in this repo the output of dts-buddy is commited, so /types/index.d.ts is what is going to get published. a mismatch is prevented by calling a script that ensures the output is in sync with the current repo content.

…nctions are properly exported through module declaration generated by dts-buddy; move dynamicCompileOptions to PluginOptions, rename SvelteOptions to SvelteConfig
Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

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

Still not a fan of dts-buddy 😬 But I won't block the PR if you want to merge it.

.changeset/metal-olives-wait.md Outdated Show resolved Hide resolved
@dominikg dominikg merged commit 62afd80 into main Oct 26, 2023
5 checks passed
@dominikg dominikg deleted the types/dts-buddy branch October 26, 2023 17:27
@github-actions github-actions bot mentioned this pull request Oct 26, 2023
@github-actions github-actions bot mentioned this pull request Nov 16, 2023
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.

3 participants