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

Using prop-types outside of React #34

Closed
jwalton opened this issue Apr 20, 2017 · 24 comments · May be fixed by #54
Closed

Using prop-types outside of React #34

jwalton opened this issue Apr 20, 2017 · 24 comments · May be fixed by #54

Comments

@jwalton
Copy link

jwalton commented Apr 20, 2017

Now that prop-types is a top-level package, it occurs to me that there are several use cases where this could be a useful library outside of React.

Use Case

Suppose, for example, you are writing an application with a REST interface that takes in some complicated JSON data, and queries MongoDB:

app.post("/api/getLastUserLogin", (req, res) => {
    // Expect req.body to be a `{username}` object.
    db.users.find({username: body.username})
    .then(users => {
        res.json(users[0] && users[0].lastLoginTime);
    });
});

Simple enough... But, oh noes! There are dark forces at work on the internet, and someone sends us this JSON body:

{"username": {"$ne": ""}}

MongoDB faithfully tries to load all 10 million of our users into memory, the server crashes, our company goes bankrupt, and our children starve.

But wait! prop-types to the rescue!

const LastUserLoginMessageType = {
    username: PropTypes.string.isRequired
}

app.post("/api/getLastUserLogin", (req, res) => {
    if(!PropTypes.validate(LastUserLoginMessageType, req.body)) {
        return res.status(400).send("Not today, hax0r!");
    }
    ...
});

Why this is awesome

There's a lot to recommend this. First, it's easy to check types without having to pull in some extra build step like TypeScript or Flow (especially nice on existing projects which don't already incorporate those technologies). Second, it's a syntax that many people are already very familiar with, because it's so widely used in React.

The challenge is, of course, that today prop-types is so tightly coupled to React. For starters, all of prop-types is disabled if NODE_ENV === 'production', which is an obvious problem, so there'd need to be some changes. Turning prop-types into a more general-purpose library should have advantages for React though, as it should make it easier to do things like #28.

API Proposal

This is just to give a vague idea of what I'm after here, there's lots of room for improvement on this, but:

PropTypes.isInvalid(typeSpec, value, options)

  • typeSpec is an object where keys are property names, and values are the associated PropType objects (e.g. {username: PropTypes.string}).
  • value is the object we're going to check.
  • options.context - See options.onValidationError().
  • options.onValidationError({path, expectedType, actualType, errorString, context}) - If this function returns a truthy value, then isInvalid() will return that value immediately. If this function throws, the the error will propagate all the way to the caller of isInvalid(). path is the path of the value being checked (e.g. "user.name"). expectedType is the type that was expected at this path (e.g. 'string'), actualType is the type we actually found. context is the options.context object passed to isInvalid(). errorString is an english-language ready made error string to describe what went wrong. The default implementation simply returns errorString.

I think this gives you flexibility to do quite a bit. The existing React use case, where we need to know if value came from props or context or whatever is solved by passing {location, componentName} along in the context, so onValidationError() can construct an appropriate string. React would set onValidationError: () => null when running production code.

One minor downside to this is it makes a webpacked production React app slightly bigger, since all this code that used to get optimized out is no longer going to be enclosed in NODE_ENV === 'production' checks. I can think of a couple of ways around that, but I'll leave that for future discussion.

@reallyimeric
Copy link

I'm using it to do validating. Currently PropTypes.checkPropTypes will not throw or return false or return rejected promise on checking failed, and this makes me hard to know if checking was failed so I write a function to do checking manually..

function customedCheckPropTypes(typeSpecs, values, location, componentName) {
  const promiseArray = Object.keys(typeSpecs).map((typeSpecName) => {
    if (typeof typeSpecs[typeSpecName] !== 'function') return Promise.reject(`${location}, ${typeSpecName}: TypeChecker should be a function`);
    const error = typeSpecs[typeSpecName](values, typeSpecName, componentName, location, null, 'SECRET_DO_NOT_PASS_THIS_OR_YOU_WILL_BE_FIRED');
    // specified using prop-types version 15.5.8 only in package.json
    if (error instanceof Error) return Promise.reject(`Failed ${location} type: ${error.message}`);
    return Promise.resolve();
  });
  return Promise.all(promiseArray);
}

Maybe checkPropTypes return false on failed can help a lot.

@jwalton
Copy link
Author

jwalton commented May 6, 2017

@reallyimeric This works, but only if you don't set NODE_ENV=production, otherwise you'll get shims in place of typeSpecs.

rufman pushed a commit to rufman/prop-types that referenced this issue May 17, 2017
Adds a `checkPropTypeWithErrors` convenience function, which calls
`checkPropTypes` with `throwErrors` set to true

Fixes facebook#34
rufman added a commit to rufman/prop-types that referenced this issue May 18, 2017
Adds a `checkPropTypeWithErrors` convenience function, which calls
`checkPropTypes` with `throwErrors` set to true

Fixes facebook#34
rufman added a commit to rufman/prop-types that referenced this issue Jul 13, 2017
When specified, the argument `warningLogger` will be called with the
same arguments as `warning`. Instead of logging errors to the fbjs
warning logger, we are able to handle them externally.

Fixes facebook#34
rufman added a commit to rufman/prop-types that referenced this issue Sep 5, 2017
When specified, the argument `warningLogger` will be called with the
same arguments as `warning`. Instead of logging errors to the fbjs
warning logger, we are able to handle them externally.

Fixes facebook#34
rufman added a commit to rufman/prop-types that referenced this issue Oct 4, 2017
When specified, the argument `warningLogger` will be called with the
same arguments as `warning`. Instead of logging errors to the fbjs
warning logger, we are able to handle them externally.

Fixes facebook#34
@lili21
Copy link

lili21 commented Aug 9, 2018

any updates ?

@reallyimeric
Copy link

For anyone looking for a validating library, ajv would be a good choice.

@dragonwong
Copy link

I want to use like this:

class Test {
  constructor(props) {
    this.scale = props.scale;
  }
}

Test.propTypes = {
  scale: PropTypes.number,
};
Test.defaultProps = {
  scale: 1,
};

const test = new Test({});

// expect: 1
console.log(test.scale);

🤧🤧🤧

@ljharb
Copy link
Collaborator

ljharb commented Nov 8, 2018

@dragonwong for that to work, the JS language itself would have to support prop types and default props. You can, however, do this:

class Test {
  constructor({ scale = 1, ...rest }) {
    this.props = { scale, ...rest };
  }
}

@dragonwong
Copy link

@ljharb thanks, but I think it can not support complex feature such as PropTypes.shape, PropTypes.oneOfType, .isRequired and etc. 😰

And prop-types can check the props, when the props is invalid it will throw a warning, I think that is really important too.

@zzzzMaDaOzzzz
Copy link

aggree with @dragonwong , I was trying to use prop-types in nodeJS for attributes validation

@ljharb
Copy link
Collaborator

ljharb commented Feb 21, 2019

I'm not sure what's still actionable here - you can certainly use this library outside React (checkPropTypes), but the language doesn't have the ability to do it for you. Closing, but happy to reopen if needed.

@ljharb ljharb closed this as completed Feb 21, 2019
@jwalton
Copy link
Author

jwalton commented Feb 21, 2019

@ljharb The use case example given in the original issue can't be done with the library, as-is. You can, indeed, call checkPropTypes to check if a JSON object matches a props description (although you need to pass in some names that really have nothing to do with checking if a JSON object matches a schema), but, from the documentation:

If you absolutely need to trigger the validation manually, call PropTypes.checkPropTypes(). Unlike the validators themselves, this function is safe to call in production, as it will be replaced by an empty function

Emphasis mine. So in my example where I'm trying to prevent someone from passing bad data via a REST API, I can verify that with prop-types on the server, but it will automatically be turned off for me in production. This is not desired behavior for a schema checking library.

@ljharb
Copy link
Collaborator

ljharb commented Feb 21, 2019

@jwalton ah - you can choose to use the development version in production, and it'll work fine https://unpkg.com/prop-types@15.7.2/prop-types.js

@jwalton
Copy link
Author

jwalton commented Feb 21, 2019

But how would I do that? NODE_ENV is set to 'production' on my production servers, because all the other node packages I use rely on it being set that way to behave sensibly in production. I'd have to do something hacky like:

const originNodeEnv = process.env.NODE_ENV;
process.env.NODE_ENV = 'development';
const propTypes = require('prop-types');
process.env.NODE_ENV = originNodeEnv;

Yuck.

@ljharb
Copy link
Collaborator

ljharb commented Feb 21, 2019

The file I linked does not check process.env; it's hardcoded to development.

@ljharb
Copy link
Collaborator

ljharb commented Feb 21, 2019

iow, instead of requiring prop-types, you can use prop-types/prop-types in your bundler.

@jwalton
Copy link
Author

jwalton commented Feb 22, 2019

If you open up node, and try:

p = require('prop-types/prop-types');
console.log(p.checkPropTypes.toString());

You'll see the code is:

function checkPropTypes(typeSpecs, values, location, componentName, getStack) {
  if ("development" !== 'production') {
    // Do a bunch of stuff
  }
}

Open up that link you posted above and find checkPropTypes and you'll see it's the same.

@ljharb
Copy link
Collaborator

ljharb commented Feb 22, 2019

Yep, and that works fine, because the string "development" will never be equal to the string "production".

@jwalton
Copy link
Author

jwalton commented Feb 22, 2019

I am not a smart man. :P Thanks, sir.

@rgraffbrd
Copy link
Contributor

@jwalton ah - you can choose to use the development version in production, and it'll work fine https://unpkg.com/prop-types@15.7.2/prop-types.js

I'd like to see this in the docs, that would be a big help :)

@ljharb
Copy link
Collaborator

ljharb commented Feb 26, 2019

a PR is welcome.

@rgraffbrd
Copy link
Contributor

@ljharb #262

@alexeychikk
Copy link

alexeychikk commented May 27, 2019

import * as PropTypes from 'prop-types/prop-types'
This doesn't work for TypeScript because internal prop-types.js file is untyped.

@ljharb
Copy link
Collaborator

ljharb commented May 27, 2019

@alexeychikk then that’s a bug in the DT package - all files should be typed, not just the main. (separately, import * there is incorrect; TS’ module system is broken unless you enable synthetic imports and es module interop)

@alexeychikk
Copy link

@ljharb TS module system is not the issue. With esModuleInterop it works both ways:

import * as PropTypes from "prop-types";
import PropTypes from "prop-types";

The problem is that type definitions in @types/prop-types are not defined for prop-types/prop-types.js.

@ljharb
Copy link
Collaborator

ljharb commented May 29, 2019

Sure, that's why i said "separately" :-) please file a bug on DT so that the types can be defined for every file.

rufman added a commit to rufman/prop-types that referenced this issue Sep 4, 2019
When specified, the argument `warningLogger` will be called with the
same arguments as `warning`. Instead of logging errors to the fbjs
warning logger, we are able to handle them externally.

Fixes facebook#34
rufman added a commit to rufman/prop-types that referenced this issue Sep 4, 2019
When specified, the argument `warningLogger` will be called with the
same arguments as `warning`. Instead of logging errors to the fbjs
warning logger, we are able to handle them externally.

Fixes facebook#34
ljharb pushed a commit to rufman/prop-types that referenced this issue Dec 23, 2021
When specified, the argument `warningLogger` will be called with the
same arguments as `warning`. Instead of logging errors to the fbjs
warning logger, we are able to handle them externally.

Fixes facebook#34
ljharb pushed a commit to rufman/prop-types that referenced this issue Dec 23, 2021
When specified, the argument `warningLogger` will be called with the
same arguments as `warning`. Instead of logging errors to the fbjs
warning logger, we are able to handle them externally.

Fixes facebook#34
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 a pull request may close this issue.

8 participants