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

fix: expose types in package #516

Closed
wants to merge 1 commit into from
Closed

Conversation

namack
Copy link

@namack namack commented Dec 13, 2023

Description

This exposes the typescript declaration files for typescript to reference when used in a project. This is the simplest way to accomplish this since the plugin type files are already included in the package. However, a more involved solution would be to add a build step for the compiled types to be included in the /dist directory.

Before this PR typescript users would run into type errors while importing the package:

image

After this PR, typescript will automatically infer the types for the exported functions/classes:

image

Checklist

  • Read the contributing guidelines.
  • Each commit follows the Conventional Commit spec format.
  • Update the readme (if applicable).
  • Update or add any necessary API documentation (if applicable)
  • All existing unit tests are still passing (if applicable).

@namack namack requested a review from a team as a code owner December 13, 2023 20:29
@luqven luqven added the enhancement New feature or request label Dec 14, 2023
@luqven
Copy link
Contributor

luqven commented Dec 14, 2023

Hey @namack, thanks for opening this PR.

I do think adding a build is preferable. It should be pretty straightforward, we can make use of vite-plugin-dts and add { rollupTypes: true }. That should be about it.

// [vite.config.js](https://github.com/imgix/vue/blob/9aebc6f129422ed21a9272f1a7e3fd9b3207794e/vite.config.js#L41)

import dts from 'vite-plugin-dts';
// ...
export default defineConfig({
  // ...
  plugins: [vue(), dts({ rollupTypes: true })],
  // generates imgix-vue.esm.d, imgix-vue.min.d
})

If you're interested in tackling this change yourself, let us know and we can assign an issue to you.

In the meantime, we'll make this change as soon as someone on the team has the bandwidth.

@luqven luqven linked an issue Dec 14, 2023 that may be closed by this pull request
3 tasks
@namack
Copy link
Author

namack commented Dec 14, 2023

@luqven Great! Yes, this is definitely a less hacky solution 😄. I don't mind implementing the change, feel free to assign an issue to me.

@luqven luqven removed a link to an issue Dec 14, 2023
3 tasks
@luqven
Copy link
Contributor

luqven commented Dec 14, 2023

Closing in favor of using build step to bundle .d.ts files into /dist

@luqven luqven closed this Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants