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: types #12

Closed
wants to merge 3 commits into from
Closed

fix: types #12

wants to merge 3 commits into from

Conversation

userquin
Copy link

@userquin userquin commented Jul 26, 2023

This PR includes:

  • add package manager to root package.json
  • add unbuild dev dependency in library package.json
  • build valibot using unbuild instead Vite: Vite still there, if you want still use it (you can remove the dependency)
  • exports all submodules
  • added dts file per submodule for node10: included in dist package entry
  • tsc will be only used to copy source files to dist folder once built
  • udpated playground script to generate also stubs

If you want to only export the index module:

  • remove all library d.ts files from library folder, remove also d.cts ones, don't remove index.d.cts
  • remove all exports from library package.json (keep only ".")
  • remove "resolveJsonModule": true from library tsconfig file: only used to read the import entry from the package exports
  • remove all code from build.config.ts file, keep only the import and the default export using defineBuildConfig (remove also the type from the import)
  • replace entries with entries: ['src/index'], in build.config.ts file
  • remove *.d.ts from library package.json dist option, don't remove *.d.cts

You can build and pack valibot: pnpm build && pnpm pack and upload generated tgz file to https://arethetypeswrong.github.io/

closes #7

@netlify
Copy link

netlify bot commented Jul 26, 2023

Deploy Preview for valibot canceled.

Name Link
🔨 Latest commit a9edcd8
🔍 Latest deploy log https://app.netlify.com/sites/valibot/deploys/64c13cd6cd7f390008710bfd

@userquin
Copy link
Author

userquin commented Jul 26, 2023

The root types and main package entries updated for old TypeScript versions support: https://www.typescriptlang.org/docs/handbook/esm-node.html#packagejson-exports-imports-and-self-referencing

imagen

@fabian-hiller
Copy link
Owner

Thanks for your research and technical implementation! What is the advantage of the submodules and the .d.ts and .d.cts files in the library directory? I have not fully understood this yet.

@fabian-hiller fabian-hiller self-assigned this Jul 26, 2023
@fabian-hiller fabian-hiller added the bug Something isn't working label Jul 26, 2023
@userquin
Copy link
Author

About submodules, here none. About d.cts and d.ts, you need to add both to allow use valibot in CJS and ESM, check typescriptlang link, TypeScript requires both.

@fabian-hiller
Copy link
Owner

Do these files have to be in the library directory or can they also be in dist?

@userquin
Copy link
Author

userquin commented Jul 26, 2023

Must be on root, ts resolution will lookup them for node10, dist folder will be regenerated on each build.

Check the resolutions entries in https://arethetypeswrong.github.io/ with the tgz file, expand the details below the table

@fabian-hiller
Copy link
Owner

fabian-hiller commented Jul 26, 2023

Would we save all the hassle if we generated a single .mjs, .cjs, .d.ts and .d.cts file from the many source files in the build step? I don't want to complicate the repository unnecessarily. Currently I am a bit averse to the many files in the library directory.

@userquin
Copy link
Author

userquin commented Jul 26, 2023

Yes, we only need the index.d.cts and the build.config.ts module, we can remove all the submodules, check first comment about removing submodules.

I'll cleanup the PR tmr.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issues importing with Node16/NodeNext moduleResolution
2 participants