Skip to content

Conversation

@voxpelli
Copy link
Collaborator

It's not enough that the JS-code itself executes and runs. As Typescript supports validation of JS-files the types needs to work for anyone who uses that (like I do).

I've in this PR added type validation of the cjs and esm code as well, and it shows that the types aren't valid in the cjs scenario, which is what was brought up by me in fastify/env-schema#16 and which @mcollina pointed towards this repo to explain.

Reason why I added the esm/package.json is that TypeScript yet doesn't like the .mjs file ending, microsoft/TypeScript#27957, so specifying that it's an esm file in the package.json is a way around that.

@fox1t
Copy link
Owner

fox1t commented Oct 16, 2020

Well done! One of the most significant missing features of this repo was that it didn't show how .mjs files are broken. Unfortunately, I haven't recently updated it.

More context here: https://twitter.com/maksimsinik/status/1316001906746064898Well done! Not showing the .mjs was one of the biggest missing feature of this repo and I haven't any time recently to update it. More context here: https://twitter.com/maksimsinik/status/1316001906746064898

@voxpelli
Copy link
Collaborator Author

@fox1t Core thing this shows though is that the types of Fastify isn't compatible with Typescript checked cjs 🙂 .mjs is a complicated topic in itself, but using JSDoc + cjs is an option gaining traction as an alternative to full on Typescript projects

@fox1t
Copy link
Owner

fox1t commented Oct 16, 2020

@fox1t Core thing this shows though is that the types of Fastify isn't compatible with Typescript checked cjs 🙂 .mjs is a complicated topic in itself, but using JSDoc + cjs is an option gaining traction as an alternative to full on Typescript projects

I am sure that we can make the old and beloved CJS work somehow. I am not sure about the new and fresh ESM, though. XD

@voxpelli
Copy link
Collaborator Author

@fox1t Updated, removed all of the things I did that touched esm and refocused on cjs with a new typed-cjs/ folder as you wanted

Can squash later if that's wanted

@voxpelli voxpelli changed the title Validate types in the cjs and esm as well (shows that its failing in cjs) Validate types against the cjs (shows that its failing) Oct 16, 2020
Copy link
Owner

@fox1t fox1t left a comment

Choose a reason for hiding this comment

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

LGTM

@fox1t fox1t merged commit f11c763 into fox1t:master Oct 16, 2020
@voxpelli voxpelli deleted the validate-types-in-js branch October 19, 2020 16:36
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.

2 participants