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

chore: add TypeScript as a dependency #2588

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

wojtekmaj
Copy link
Contributor

@wojtekmaj wojtekmaj commented Jan 31, 2024

This PR does NOT migrate the codebase to TypeScript nor indicates intent to do so in the future. What it does is simply installs TypeScript, so that developer experience (when writing JavaScript!) can be improved.

On top of that, this makes yarn tsc command available. At the moment, it still produces a lot of errors, so it's pretty useless, but as #2587 (and the PRs that come after that) gets merged, slowly but surely we'll have increased type safety.

Eventually, we could consider having type checking as a CI step to ensure better quality. Still, writing JavaScript!

Copy link

changeset-bot bot commented Jan 31, 2024

⚠️ No Changeset found

Latest commit: 8663661

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@diegomura
Copy link
Owner

Needs small rebase (sorry 😄 ) . @wojtekmaj I'm open to this, but not sure I see the usage if we aren't migrating to TS. Can you expand on how DX get's improved?

@wojtekmaj
Copy link
Contributor Author

wojtekmaj commented Feb 5, 2024

TypeScript helps you even if you're not writing TypeScript. There are a couple things it helps you out with:

Built-in declaration

Type safety when using browser APIs

No TS - this typo would have been caught at runtime:

image

With TS - all red!

image

Type safety when using Node.js APIs

You get some of that using VSCode out of the box, but there are some things it can't help you with without proper TypeScript (tool, not language) setup. For example, what's available in Node.js differs vastly from version to version. So types for Node have to be installed manually to get DX improvements related to Node APIs.

This actually saved my butt today at work:

image

I did not know File global was not available in Node.js 18 (until something like the 15th minor version, can't be bothered to check exactly), and while I was happily coding in Node.js 20, everything was running smoothly, our Node.js 18-based server would have crashed.

This is even more true for react-pdf, where we don't know which version of Node the users are going to run, and forcing you to run Node.js, say, 16 just for the sake of ensuring it will run would be ridiculous.

Note on the above

Perhaps even more important than types is the lack of it. Because we can tell TypeScript what environment we're targetting, it can tell you, for example, that you're using browser APIs when you're writing code intended for the server side.

@types/*

You may add @types/* packages for dependencies you use that do not ship with their own types. You get stuff like inline documentation, autocomplete in many cases, etc.

JSDoc

TypeScript understands JSDoc. So my efforts to document internals of react -pdf will eventually pay off by improved DX (just like with @types/* - see above), and - with all due respect, I guarantee it - by bugs it's gonna find.

And…

No @types/*, no JSDoc? There are still inferred types you can benefit from.

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.

None yet

2 participants