Skip to content

pass emit as an argument#127

Closed
calvinmetcalf wants to merge 1 commit intoapache:masterfrom
calvinmetcalf:pass-arguments
Closed

pass emit as an argument#127
calvinmetcalf wants to merge 1 commit intoapache:masterfrom
calvinmetcalf:pass-arguments

Conversation

@calvinmetcalf
Copy link

allow the map function to be passed a second argument, the emit function. Benefits as I see them

  • 100% backwards compatible, emit is still in the execution closure.
  • makes it possible to run views in pure JavaScript without having to use eval (e.g. in pouchdb)
  • allows emit to be renamed in the case of some sort of collision, the circumstances of which I can't imagine.

I can write tests if this is something worth pursuing, though at the moment I'm having troubling fathoming the ruby wrapped JavaScript testing format.

Thanks.

@kxepal
Copy link
Member

kxepal commented Jan 22, 2014

I'm not sure that this change is reasonable. Yes, it makes views code become less depended on global functions, but emit isn't the only that available: see sandbox for all available context. So even if you allow to redefine the emit, some views may use toJSON, isArray, log and sum so you still have to run such views within context where these functions are available - I'm not sure that you'd like to see all of them as function arguments.

@calvinmetcalf
Copy link
Author

sum is technically the only one you need, isArray (Array.isArray), toJSON
(JSON.stringify?), and log (console.log and/or Math.log) are supplied by
other environments. But really emit is the only mandatory one, PouchDB
only has emit and sum and literally nobody has complained (I'm probably
going to open an issue on that though now). A better idea for the other
functions might be to execute the map function bound so that the this
value has sum, toJSON, isArray, sum, and log as methods.

On Tue, Jan 21, 2014 at 7:05 PM, Alexander Shorin
notifications@github.comwrote:

I'm not sure that this change is reasonable. Yes, it makes views code
become less depended on global functions, but emit isn't the only that
available: see sandboxhttps://github.com/apache/couchdb/blob/master/share/server/loop.js#L19for all available context. So even if you allow to redefine the
emit, some views may use toJSON, isArray, log and sum so you still have
to run such views within context where these functions are available - I'm
not sure that you'd like to see all of them as function arguments.


Reply to this email directly or view it on GitHubhttps://github.com//pull/127#issuecomment-32979290
.

-Calvin W. Metcalf

@garrensmith
Copy link
Member

I like it from a best practices stand point. Its a little cleaner to pass the emit in. However other than making Pouchdb not need eval I don't really see the benefit.

@calvinmetcalf
Copy link
Author

my other thought had been so you could rename emit, like if you were
reusing code for something. I could also see it as a pattern in general
which would make it easier to write a query server for a language that
doesn't support closures.

On Wed, Jan 22, 2014 at 2:31 AM, garren smith notifications@github.comwrote:

I like it from a best practices stand point. Its a little cleaner to pass
the emit in. However other than making Pouchdb not need eval I don't
really see the benefit.


Reply to this email directly or view it on GitHubhttps://github.com//pull/127#issuecomment-32999121
.

-Calvin W. Metcalf

@wohali
Copy link
Member

wohali commented Mar 19, 2017

Hey @calvinmetcalf I'm going to close this PR as there doesn't seem to be consensus.

If you'd like to pursue this still, please open a JIRA issue and let's see if we can get some more eyes on the proposal.

@wohali wohali closed this Mar 19, 2017
nickva pushed a commit to cloudant/couchdb that referenced this pull request Apr 21, 2017
This closes apache#127

Signed-off-by: Eric Avdey <eiri@eiri.ca>
janl pushed a commit that referenced this pull request Jan 5, 2020
Update documentation for require_valid_user
nickva pushed a commit to nickva/couchdb that referenced this pull request Sep 7, 2022
Update documentation for require_valid_user
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.

4 participants

Comments