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

Runtime validation API in "development mode" #2153

Closed
kentcdodds opened this issue Nov 23, 2016 · 10 comments
Closed

Runtime validation API in "development mode" #2153

kentcdodds opened this issue Nov 23, 2016 · 10 comments

Comments

@kentcdodds
Copy link

Inspired by conversations in this PR: #2152

Would be cool to do full runtime validation like React does (it uses invariant). PropTypes is one of the most valuable things available from React. As cool as static type checking is, runtime checking will always be more powerful (and works for anyone regardless of whether they're using a type checker or not).

I created api-check to do this for one of my libraries and it really reduced the number of beginner issues I received. Everyone was happier.

Definitely would want to have the ability to exclude this runtime checking in the production code.

@jayphelps
Copy link
Member

Moving these comments to this thread:

Could we avoid introducing any code like this that's going to require a specific build process? We have a ton of consumers of Rx not using Webpack, so I'd like to avoid penalizing them.
@robwormald


Yes, that would be best. Optimally we should probably do what React's done by having the minified version exclude this stuff. So people not using a build system should use the minified version, and people using a build system can remove that themselves as part of the build. I wonder if there's a better way...

If I recall correctly, jmdobry had a lib-name.debug.js file for his js-data library which you were supposed to use in development which had helpful things like this and then in production you'd use (and minify yourself) lib-name.js. Seems legit to me.

All that said, one of the drawbacks of doing an "opt-in" system for this kind of thing is the people who generally need this kind of help wont know to opt into the help and you lose one of the main benefits of doing something like this in the first place. I think that making an opt-out version which removes this stuff is better.
@kentcdodds

@trxcllnt
Copy link
Member

It would be neat (and probably insanely slow) if TypeScript had an option to compile its static type-checks to runtime code for debug builds.

@kentcdodds
Copy link
Author

It's a good idea for this use case, but based on my conversations with the team that'll probably never happen :-(

@robwormald
Copy link
Contributor

@trxcllnt its hypothetically possible i would think with the upcoming transforms functionality for TS - see microsoft/TypeScript#6508 for tracking on that

RxJS is designed to be modular (cuz its got loads of operators), so there isn't a single place we could swap this out - most (angular) consumers are doing something like:

import {Observable} from 'rxjs/Observable'
import 'rxjs/add/operator/mergeMap'

rather than using a single bundle, you can get the bits and pieces you want.

I'm 👍 on improving approachability / debuggability, just not at the expense of performance or needing special handling for prod.

As we already monkey patch the world, perhaps you could optionally import a file like rxjs/debug which (re)monkeypatches operators with type assertions?

cc @alexeagle

@jayphelps
Copy link
Member

jayphelps commented Nov 30, 2016

@robwormald just to clarify, the initial proposal was using process.env.NODE_ENV checks like React (and others) use, which webpack (and others) easily strip out. https://facebook.github.io/react/docs/optimizing-performance.html

So it's no different whether they use operator patching or not.

We would also distribute builds that do not include these checks, for UMD usage. We could also try to detect prod usage of dev builds and log a warning, again like React does. (i have no idea how, but I imagine I could quickly find)

@kwonoj
Copy link
Member

kwonoj commented Apr 16, 2017

I've changed topic bit to use as umbrella issue for runtime validation issues.

@jayphelps
Copy link
Member

jayphelps commented Oct 15, 2017

@kentcdodds IMO this is a good idea still. Any objection to keeping it open? It’s been almost a year, which is prolly why you closed it, but it’s still valid and something I think should be considered when other more critical things are completed

@kentcdodds
Copy link
Author

Agreed it's still important. I was just wasting my time closing my old issues that I no longer care enough to put more work into 😅

@kentcdodds kentcdodds reopened this Oct 15, 2017
@jayphelps
Copy link
Member

@kentcdodds Right on. Definitely feel free to unsubscribe 👍

@benlesh
Copy link
Member

benlesh commented Aug 18, 2020

In general, more run time type checking could/should be done. However we'd probably need something more bespoke and compact, if we were going to use a more unified approach. I'm going to close this for now, as it seems that the library is mostly in a good place for this. But will monitor for other issues.

@benlesh benlesh closed this as completed Aug 18, 2020
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

No branches or pull requests

6 participants