Skip to content
This repository has been archived by the owner on Mar 13, 2018. It is now read-only.

Do not create excess globals; consider collecting APIs under a namespace as needed #160

Closed
sjmiles opened this issue Feb 26, 2014 · 6 comments

Comments

@sjmiles
Copy link
Contributor

sjmiles commented Feb 26, 2014

I don't actually know for sure which libraries are doing this, but I wanted to capture the issue without doing the due diligence (sorry!).

In general, we like to be very circumscribed about the global namespace and create as few globals per new API as possible.

@rafaelw
Copy link
Contributor

rafaelw commented Feb 26, 2014

Presumably you're currently looking at offending global names. Can you list them?

@sjmiles
Copy link
Contributor Author

sjmiles commented Feb 26, 2014

I'm not currently looking, that's the bit about the missing due diligence. But this is one of those things that was rattling around in my bean for a long time so I just wanted to get something down into issues.

@jmesserly
Copy link
Contributor

fyi, I don't think TemplateBinding creates globals (it just patches HTMLTemplateElement and things like that) ... however, I did noticed recently that observe-js has quite a few globals:

https://github.com/Polymer/observe-js/blob/9ac422a456bcbaf5cf61b966d29c55088ff57e9e/src/observe.js#L1589

(I was actually happy to discover this, since it makes it possible for me to prototype JS+Dart data binding interop without needing any patches to platform.js ... for example I could patch PathObserver and I think TemplateBinding would pick up the patched version. I didn't actually try yet though, and not sure if ultimately that should be considered a feature... ;-) )

@sjmiles
Copy link
Contributor Author

sjmiles commented Feb 26, 2014

Ok, my bad if I put this issue in the wrong place.

Let's be clear that surfacing an API is not the same as creating a global. I'm not saying we need to privatize more API, just that we should collect these things into namespace objects.

@jmesserly
Copy link
Contributor

no worries :). agreed that namespacing is good!

(minor note: it does tend to make it impossible to patch the constructor itself, for example, if this is TemplateBinding.js:

(function(scope) {
  var Path = scope.observe.Path;
  var CompoundObserver = scope.observe.CompoundObserver;
  ...
})(...);

...since observe-js is compiled with TemplateBinding into the same platform.js, it's not possible to swap out the "CompoundObserver" constructor anymore. Whereas if it's always looked up from "window" I could do that. But I yeah, it's not worth messing up the API design for that reason :). And always qualifying it like observe.Path would be ugly. Anyway, forget I brought this up :) )

@jmesserly
Copy link
Contributor

moved to googlearchive/observe-js#66

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

No branches or pull requests

3 participants