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

(#2158) - bring back custom extend #2270

Merged
merged 1 commit into from
May 25, 2014
Merged

(#2158) - bring back custom extend #2270

merged 1 commit into from
May 25, 2014

Conversation

nolanlawson
Copy link
Member

OK, so I figured out that the Ember.js bug was caused by the change from using a custom extend method (ripped from jquery) to the one on npm. Proof here: http://bl.ocks.org/nolanlawson/7bb46ec8310196357217

I haven't figured out yet why the change caused the error, but I think the important thing is that we get Ember.js support back for the 2.3.0 release. (Yes, users can always disable the prototype modification in Ember.js, but they won't. Or they will, but only after some Googling and much frustration, at which point they will blame PouchDB for their troubles and we'll have lost a user.)

@calvinmetcalf
Copy link
Member

Can you turn this into either a seperate module or a pull on the other extend.

@nolanlawson
Copy link
Member Author

I'll try, but I'm not sure I'll be able to before the 2.3.0 release. Hence this monkey patch.

@nolanlawson
Copy link
Member Author

Also, I'd rather not pollute npm with a module that kinda sorta works and I'm not sure why. It's only 94 lines of code, so I don't see the harm in just including it. If you'd like to make a module and depend on it, though, then be my guest. I also filed an issue in node-extend.

@calvinmetcalf
Copy link
Member

Don't worry about npm, I've got some 8 line modules
On May 22, 2014 8:33 PM, "Nolan Lawson" notifications@github.com wrote:

Also, I'd rather not pollute npm with a module that kinda sorta works and
I'm not sure why. It's only 94 lines of code, so I don't see the harm in
just including it. If you'd like to make a module and depend on it, though,
then be my guest. I also filed an issue in node-extend.


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

@nolanlawson
Copy link
Member Author

Fair enough, the deed is done.

@ssured
Copy link
Contributor

ssured commented May 24, 2014

Ping to stay up to date with this merge. Hit this issue tooo

@calvinmetcalf
Copy link
Member

@nolanlawson good except you need to remove the old version as well

@nolanlawson
Copy link
Member Author

Whoops, fixed.

@calvinmetcalf
Copy link
Member

👍 if green

@nolanlawson nolanlawson merged commit 25b481a into master May 25, 2014
@nolanlawson nolanlawson deleted the 2158 branch May 25, 2014 06:47
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.

None yet

3 participants