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.

Sign up for GitHub

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

Jump to bottom

Remove delete #45

Open
michaelBenin opened this issue Feb 2, 2015 · 5 comments
Open

Remove delete #45

michaelBenin opened this issue Feb 2, 2015 · 5 comments
Labels
enhancement

Comments

@michaelBenin
Copy link
Collaborator

https://github.com/lukasolson/backbone-super/blob/master/backbone-super/backbone-super.js#L57

Delete is a major perf hit:

http://kendsnyder.com/chrome-v8-creators-with-and-delete-are-dog-slow/
@philfreo
Copy link
Collaborator

philfreo commented Feb 2, 2015

What do you suggest instead?

@michaelBenin
Copy link
Collaborator Author

So before I found this project I was using:

https://coderwall.com/p/xj81ua/a-better-way-of-extending-backbone-models-and-views

And fixed it in my hobby project:

https://github.com/michaelBenin/node-startup/blob/master/shared/utils/util.js#L5

If this was added would be a major version bump.

@apsavin
Copy link
Contributor

apsavin commented Feb 4, 2015

@michaelBenin Can you provide a benchmark that it's really affect us in this particular case?

@michaelBenin
Copy link
Collaborator Author

@apsavin yes a perf comparison on the pr that removes the delete would be useful.

@michaelBenin
Copy link
Collaborator Author

@lukasolson when you get a chance maybe you could weigh in here.

michaelBenin added a commit to michaelBenin/backbone-super that referenced this issue Feb 9, 2015
@michaelBenin
Issue lukasolson#45 - removed delete, refactored plugin
a52a0f5 
Issue lukasolson#44 - fnTest is way too broad

This commit refactored the project and fixed one bug, where
naming a dataType _super caused issues. See: lukasolson#44

Also we refactored the structure of this project. One thing I'd like some feedback on is using the `for in` loop. I'm curious about the projects status working
x browser.

- Fix: Removed delete
- Fix: allowed for dataTypes to be named _super
- Add: Documentation, refactoring

TODO: Perf testing before and after, add more tests that run in browser as
that is most likely where this library is used.
@michaelBenin michaelBenin mentioned this issue Feb 9, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@philfreo @michaelBenin @apsavin