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

Automatic node finding (this.$) can blow up with existing code #442

Closed
ebidel opened this issue Mar 10, 2014 · 6 comments
Closed

Automatic node finding (this.$) can blow up with existing code #442

ebidel opened this issue Mar 10, 2014 · 6 comments
Assignees
Labels

Comments

@ebidel
Copy link
Contributor

ebidel commented Mar 10, 2014

Not a bug in Polymer per se, but I spent a lot of time scratching my head tonight. Maybe we can prevent future grief.

The HTML5 slide deck I'm using defines HTMLElement.prototype.$:

window.$ = function(selector, opt_scope) {
  var scope = opt_scope || document;
  return scope.querySelector(selector);
};

HTMLElement.prototype.$ = function(selector) {
  return $(selector, this);
};

Requirejs loads this code before the polymer import. This means base.js picks up the $ function instead of creating it's empty hash:

marshalNodeReferences: function(root) {
  // establish $ instance variable
  var $ = this.$ = this.$ || {};

and thus, this.$ went bonkers when elements used it.

@addyosmani
Copy link
Member

Is a short term 'fix' to document that you should either avoid overriding $ or ensure that Polymer is loaded in the correct order to avoid things going cray?

@sjmiles
Copy link
Contributor

sjmiles commented Mar 10, 2014

Can you be more specific about the nature of the bonkers. Specifically, it's not obvious how having $ be a Function would cause any particular failure. IOW, a Function is also an Object.

HTMLElement.prototype.$ = ...

This is an outright name collision, so long-term I don't know how we avoid this problem. Any library that wants to put something on HTMLElement.prototype will potentially collide with some Custom Element.

@sjmiles
Copy link
Contributor

sjmiles commented Mar 10, 2014

Oh, Steve points out there was only one instance of $ for all elements. Yes, that would cause bonkers. =P

@ebidel
Copy link
Contributor Author

ebidel commented Mar 10, 2014

That's right.

var $ = this.$ = this.$ || {}; -> var $ = this.$ = {}; would do the trick, but that might mess things up for extended elements.

@sjmiles
Copy link
Contributor

sjmiles commented Mar 10, 2014

It's easy to ensure our $ wins, the problem is when whoever installed his own $ tries to use it.

@sjmiles
Copy link
Contributor

sjmiles commented Aug 18, 2014

Setting HTMLElement.prototype.$ is an anti-pattern. There is no way to solve this completely under those conditions.

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

No branches or pull requests

3 participants