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

3.0.0 plan #5

Closed
fengmk2 opened this issue Apr 14, 2015 · 23 comments
Closed

3.0.0 plan #5

fengmk2 opened this issue Apr 14, 2015 · 23 comments
Assignees

Comments

@fengmk2
Copy link
Member

fengmk2 commented Apr 14, 2015

request.query and request.queries

Make request.query[key] return strict String value and add request.queries[key] to return Array value.

  • delegate ctx.query to request.query
  • delegate ctx.queries to request.queries

request.query[key]

return String value

  • if key not exists, return undefined
  • foo[bar][baz]=foobarbaz => { 'foo[bar][baz]': 'foobarbaz' }
    • request.query.foo return undefined
    • request.query['foo[bar][baz]'] return 'foobarbaz'
  • a[]=b&a[]=c => { 'a[]': 'b' }
    • request.query.a return undefined
    • request.query['a[]'] return b
  • a=b&a=c => { 'a': 'b' }
    • request.query.a return b

request.queries[key]

return Array value

  • if key not exists, return undefined
  • foo[bar][baz]=foobarbaz => { 'foo[bar][baz]': [ 'foobarbaz' ] }
    • request.queries.foo return undefined
    • request.queries['foo[bar][baz]'] return [ 'foobarbaz' ]
  • a[]=b&a[]=c => { 'a[]': [ 'b', 'c' ] }
    • request.queries.a return undefined
    • request.queries['a[]'] return ['b', 'c']
  • a=b => { 'a': [ 'b' ] }
    • request.queries.a return ['b']
  • a=b&a=c => { 'a': [ 'b', 'c' ] }
    • request.queries.a return ['b', 'c']
@fengmk2 fengmk2 self-assigned this Apr 14, 2015
@fengmk2
Copy link
Member Author

fengmk2 commented Apr 14, 2015

/cc @dead-horse @jonathanong

@fengmk2
Copy link
Member Author

fengmk2 commented Apr 14, 2015

I don't want to write codes like these any more.

var val = this.query.foo;
if (Array.isArray(val)) {
  // ...
} else {
  // ...
}

@jonathanong
Copy link
Member

What are you going to use as a query string parsing library? I think @dougwilson and i had plans for a new query string parser.

I personally wish it was always a string. If you want types, you should be using JSON!

@fengmk2
Copy link
Member Author

fengmk2 commented Apr 14, 2015

@jonathanong yep, I personally only using string value. But other users want more value type, and I want to help them use querystring more easy and safely.

this.query[key] change to be always a string.

@ruimarinho
Copy link

Really nice @fengmk2. Wholeheartedly agree that this.query[key] returning an array is really an unsafe default operation. To be honest though, I don't really like the queryArray method name. What about this.query(key) and this.query(key, { array: true })? Also looks like a good candidate for a Map :)

Btw, reminds me of https://blogs.dropbox.com/developers/2015/03/json-in-urls/.

@dougwilson
Copy link

@jonathanong yea, I would like to get a query parsing lib in pillarjs at some point. So far I've been riding on qs, but it's been meh i.m.o :)

@fengmk2
Copy link
Member Author

fengmk2 commented Apr 15, 2015

@ruimarinho current this.query is a getter, I don't want to change it to a method.

How about this.getQuery(key, options) or this.queryValue(key, options)

@dougwilson new qs will release soon ?

@hotoo
Copy link

hotoo commented Apr 15, 2015

What about this.queryList or this.queries ? I think is not good enough, but better than this.queryArray.

@dead-horse
Copy link
Member

I don't like this.queryArray either

@fengmk2
Copy link
Member Author

fengmk2 commented Apr 15, 2015

@dead-horse @hotoo @ruimarinho how about this this.getQuery(key, {type: array|string|object})

@jonathanong
Copy link
Member

i'm guessing you guys want to make a module first, right? Something like:

const query = Query(this.querystring);
let value = query.get(key, options);

@fengmk2
Copy link
Member Author

fengmk2 commented Apr 15, 2015

@jonathanong yep, for the experienced node.js developer he/she can use module way. But most new node.js developers inside my team, I need to make sure they use a same way and don't want them to care about which version of Query module they need to use.

let value = this.getQuery(key, options);

@hotoo
Copy link

hotoo commented Apr 15, 2015

I think this.queryObject and this.getQuery(key, {type:object}) is awful.

// foo[bar][baz]=foobarbaz => this.queryObject.foo return { bar: { baz: 'foobarbaz' } }
// developer write code following:
var baz = this.queryObject.foo.bar.baz;
// if there not has `foo[bar][baz]` key in querystring, will throw javascript exception.

// developer must write code like following for safe:
var baz;
if (this.queryObject.foo && this.queryObject.foo.bar) {
  baz = this.queryObject.foo.bar.baz;
}
if (typeof baz !== "undefined") {
  // dosomething.
}

@hotoo
Copy link

hotoo commented Apr 15, 2015

Just provide this.query.key (for String) and this.queries.key (for Array) is enough and Turing-complete.

@hotoo
Copy link

hotoo commented Apr 15, 2015

  • this.request.query <-- this.query
  • this.request.queries
  • this.request.body
  • this.request.bodies

@fengmk2
Copy link
Member Author

fengmk2 commented Apr 15, 2015

@hotoo your proposal look better 👍

@jonathanong
Copy link
Member

let's get this started! https://github.com/pillarjs/qs-strict

@DenisIzmaylov
Copy link

Could you please implement middleware pattern?
To use this package in follow way:

app.use(qs('extended'));

@shannon
Copy link

shannon commented Oct 24, 2015

👍 for the middleware pattern. It would allow it to be configured in a koa-router route instead of at the root.

@itsnotvalid
Copy link

Any plans to upgrade the version for qs?

@adoyle-h
Copy link

request.query[key] return String value

@fengmk2 I think it is unacceptable. I prefer to use this.getQuery(key, {type: array|string|object}).

There is a scenario that any middleware could change the query for its business. For example, request.query.one = 1 to add a Number 1 to query, and then we will get a String '1' when access request.query.one;

@fengmk2
Copy link
Member Author

fengmk2 commented Dec 22, 2016

@adoyle-h No, request.query should be readonly in the whole request process.

@niftylettuce
Copy link
Contributor

Closing, feel free to reopen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests