search

phetsims / query-string-machine

Query String Machine is a query string parser that supports type coercion, default values & validation. No dependencies.
MIT License
3 stars 3 forks source link

Add support for required query parameters #7

Closed samreid closed 7 years ago

samreid commented 8 years ago

In some wrappers, some query parameters are required and the application should fail with a nice warning message if the query parameter is not supplied. (For instance ?sim in many of the sample wrappers). We should add support for required query parameters. Instead of providing defaultValue, it could have required: true or just omit defaultValue, which would imply that support is required.

samreid commented 8 years ago

Currently a parameter in the schema which has no defaultValue and is not supplied in the query string yields: Query String Machine Assertion failed: Value cannot be determined. It would be better if this said: Query String Machine Assertion failed: query parameter must be supplied for '+keyName'.

samreid commented 8 years ago

I decided the API will be simplest if lack of a default value in the schema implies that the value is required. I improved the error message above and added automated tests for the new behavior. If a required query parameter is missing, it will throw an error and print a message like this one:

Query String Machine Assertion failed: missing value for "sim"
samreid commented 8 years ago

@pixelzoom can you review this as part of #2 ?

pixelzoom commented 8 years ago

@samreid said

I decided the API will be simplest if lack of a default value in the schema implies that the value is required.

With one exception: schemas of type 'flag'. And that appears to work. But shouldn't providing defaultValue for type 'flag' technically be treated as an error?

samreid commented 8 years ago

With one exception: schemas of type 'flag'. And that appears to work. But shouldn't providing defaultValue for type 'flag' technically be treated as an error?

I decided to allow flag defaults to be overriden. For example, we would like the scenery "webgl" query parameter to have the following values:

?webgl=true  => true
?webgl => true
?webgl=false => false
(empty string) => true

So while the default flag behavior seems good for the majority of cases, it seems there are cases where we will want to override the default.

pixelzoom commented 8 years ago

Got it, thanks for the clarification, and I now see that logic in parseElement (line 208).

But just to confirm... Lack of a default value in the schema implies that the value is required, except for type 'flag'. Correct?

samreid commented 8 years ago

It appears the behavior is not as I prescribed in the previous comment. Assigned to me for investigation.

samreid commented 8 years ago

But just to confirm... Lack of a default value in the schema implies that the value is required, except for type 'flag'. Correct?

Yes, that's true.

pixelzoom commented 8 years ago

👍 Closing.

samreid commented 8 years ago

Reassigned to me to investigate https://github.com/phetsims/query-string-machine/issues/7#issuecomment-246853497

samreid commented 7 years ago

Moved outstanding work to side issue, closing.