search

florian-lefebvre / astro-integration-kit

A package that contains utilities to help you build Astro integrations.
https://astro-integration-kit.netlify.app
MIT License
56 stars 8 forks source link

fix: specify included files and add returns #68

Closed totto2727 closed 7 months ago

totto2727 commented 7 months ago

My project dependency included this package. When I ran tsc on that project, it reported the following type error. Disabling "noImplicitReturns": false, resolved the error, but I would like to keep the project strictly configured if possible.

I have also identified a few other type errors around testing, which we are fixing together.

21:40 Z ❯ nr check:tsc 
$ tsc --noEmit
node_modules/astro-integration-kit/src/utilities/add-virtual-import.ts:15:3 - error TS7030: Not all code paths return a value.

15 resolveId(id) {
 ~~~~~~~~~

node_modules/astro-integration-kit/src/utilities/add-virtual-import.ts:20:3 - error TS7030: Not all code paths return a value.

20 load(id) {
 ~~~~

Found 2 errors in the same file, starting at: node_modules/astro-integration-kit/src/utilities/add-virtual-import.ts:15
netlify[bot] commented 7 months ago

Deploy Preview for astro-integration-kit ready!

Name Link
Latest commit 469583a84ebda6ca60eb628ceb3ea9ef36539a93
Latest deploy log https://app.netlify.com/sites/astro-integration-kit/deploys/65df479cc8abe100082ca5f5
Deploy Preview https://deploy-preview-68--astro-integration-kit.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

florian-lefebvre commented 7 months ago

Probably same issue as https://github.com/withastro/starlight/discussions/983, as pointed by HiDeoo on Discord

totto2727 commented 7 months ago

Since the tsconfig.json is different depending on the environment, it is likely to be less problematic to transpile it.

totto2727 commented 7 months ago
  • About tests, I think it's fine if we have some type issues because they're very WIP. However, I think we shouldn't ship them to npm. Can you instead update the files field of package.json so that the tests dir is not included?
  • Can you add a changeset? pnpm changeset (patch)

DONE!

florian-lefebvre commented 7 months ago

About transpiling, I don't think we'll do this just yet with astro-integration-kit. Eventually, it will be part of astro itself (or an official package) and we'll do transpiling at that point