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

New principle: Avoid adding (non-constructor) functions to the global scope #426

Open
LeaVerou opened this issue Mar 15, 2023 · 13 comments
Open
Labels
Agenda+ tc39-tracker Issues where TC39 input would be useful Topic: JS

Comments

@LeaVerou
Copy link
Member

LeaVerou commented Mar 15, 2023

This was brought up in our discussion of Native File System API w3ctag/design-reviews#390 (comment)
For context, this API adds three methods to the global scope:

  • showOpenFilePicker()
  • showDirectoryPicker()
  • getSandboxedFileSystem()

There has also been a bunch of other methods recently added to the global scope even though they would fit better somewhere else, such as:

We do have some guidance on namespaces here but not quite a principle, which led to the confusion in the design review linked above.

IMO adding three related API methods to the global scope should have never happened. Namespaces are not just for avoiding naming collisions, they also tie an API together, and allow easier exploration. When authors see a global method like that, they have no idea where it comes from, what other methods are there, which method is part of the same API as another method, whether it's author-defined or native to the platform etc, without looking it up in documentation. It also tends to produce clunkier names as the namespace effectively becomes part of the name.

Our guidance in that design review was:

we don't have a strong opinion on whether this should be namespaced or in global. Since the amount of APIs being added is moderately low, it does seem fine to be on global - unless you have plans to add a lot more related to this capability in the future.

However, APIs tend to expand even when their designers don't initially have "plans" on expanding them, therefore namespacing should be the default, and IMO a feature should need very strong justification for adding methods to the global scope.

Note that this is about avoiding new non-constructor functions on the global scope (e.g. functions like setTimeout(), parseInt(), isNaN() etc). It is not about avoiding new constructor functions on the global scope, which are fine IMO.

@LeaVerou LeaVerou changed the title New principle: Namespace New principle: Avoid adding methods to the global scope Mar 15, 2023
@annevk
Copy link
Member

annevk commented Mar 16, 2023

By namespacing, do you mean putting them as static methods on some interface (that is itself exposed on the global)?

I don't really see the problem with structuredClone() personally. In similar vein we added qeueuMicrotask(), reportError(), and crossOriginIsolated. Not all APIs need to be scoped.

cc @domenic

@LeaVerou
Copy link
Member Author

LeaVerou commented Mar 16, 2023

By namespacing, do you mean putting them as static methods on some interface (that is itself exposed on the global)?

Yes.

Not all APIs need to be scoped.

I listed some reasons in my first post why I think they do; do you have any thoughts on why they don't?

In similar vein we added queueMicrotask(), reportError(), and crossOriginIsolated.

Adding functions to the global scope does not prove that they didn't need to be scoped — that's a cyclical argument ("the rule is not needed because look, we didn't do it here").

It's much easier to understand what a method is supposed to do when it's grouped under a shared interface.
The web platform should lead by example in terms of authoring practices, and we'd never encourage authors to add methods to the global scope like that, would we?

Consistency is another reason: TC39 seems to generally be moving away from global functions. Newer methods that improve on old globals tend to hang on a namespace: e.g. Number.isNaN() vs isNaN(). There are even things like Number.parseInt() and Number.parseFloat() that have identical functionality as their global counterparts and their sole purpose is modularization of globals.

@annevk
Copy link
Member

annevk commented Mar 16, 2023

I find it pretty clear what the methods I cited do and I don't think they would be helped by namespacing them in some manner. Sometimes namespacing might well help, but I don't think there's an easy rule.

@bathos
Copy link

bathos commented Mar 16, 2023

Operations like reportError and attributes like crossOriginIsolated do pertain to the (global) interfaces of which they’re members — unlike isNaN, say, reportError is meaningfully a “method” of global EventTarget interfaces and window.reportError.call(1, 2) must throw to be sound. Some global interface operations (e.g. atob) don’t really need this branding characteristic, though, and I agree that it’s weird for them to be defined as operations of interfaces which they have nothing to do with just for “globalness”.

@domenic
Copy link
Member

domenic commented Mar 17, 2023

I'm pretty strongly -1 on this. In addition to the reasons @annevk and @bathos mentioned:

  • foo.bar() is not really better than fooBar(); shared prefixes without a dot are just as good as shared prefixes with a dot.
  • This encourages the proliferation of non-constructible singleton classes, which is against other design principles: 1, 2.
  • TC39's design choices in this space are not a good precedent; their duplicative functions cause developer confusion for no real benefit, and their namespacing of things (e.g. under Intl or Temporal) is actively in conflict with how we do things on the web (e.g., we didn't namespace DOM.Node, DOM.Element, DOM.Attr).
  • The equivalence between web platform functions and author functions is false. The global namespace is the web platform's territory; ever since the failure of built-in modules, this has become even more clear. Author code belongs in modules, but the web platform's standard library lives on the global object. This is true whether or not those standard library functions have a . in their name, or not.
  • Guidance like this often leads people to putting their functions under document or navigator, when they have nothing to do with Documents or with user agent data. I've personally had to advise several teams to move away from such designs in their proposals. (navigator is particularly polluted these days, unfortunately. A design principle discouraging such pollution, and encouraging using the global object instead, would be great!)
  • The idea of reserving a namespace "just in case" you need it later in an API expansion often leads to bad APIs, e.g. this week I've had to advise a team to avoid window.tokenAttestation.getStateToken() (no other methods) in favor of window.getAttestationStateToken().

@bathos
Copy link

bathos commented Mar 17, 2023

In my experience narrow-purpose APIs benefit a lot from the discoverability that @LeaVerou described. It isn’t usually a problem for sets of related interfaces because their names typically share a common prefix that hints at the connection and makes it easy to see them side by side in tooling. This isn’t true for global operations, though, so they do sometimes seem to just be swimming out there — I have to look up “showOpenFilePicker” in the spec every time, for example, since the word I remember is “file”, not “show”.

Is there another way to improve/promote discoverable naming of global operations along those lines without using namespacing objects, maybe?

@LeaVerou LeaVerou changed the title New principle: Avoid adding methods to the global scope New principle: Avoid adding (non-constructor) functions to the global scope Mar 29, 2023
@syg
Copy link

syg commented Mar 29, 2023

As a TC39 delegate (i.e. this is my opinion which I also bring to TC39, but it doesn't have TC39 consensus), I find the discoverability argument somewhat compelling but as @domenic points out, the difference between foo.bar and fooBar is not a big deal. Naming consistency that aid discoverability can be had in other ways than putting things in objects.

The discussion I've seen most often come up in TC39 around namespacing things is for reduction of collisions and "pollution". IME globals are fine, and the handwringing around namespace collisions is misguided. I've only observed it to be bad ex ante. Ex post I really haven't seen it to be a problem. Where JS has had serious collision problems has been in the prototypes, specifically Array.prototype, but not in the global.

Implementation-wise, I think namespace objects do offer the benefit of making lazy loading more pay-for-what-you-use, since the indirection via the namespace object means you can lazy load the entire object. If you had consistently prefixed global functions, you'd still need a slot on the global for the name of the function, even if you choose to defer other aspects (like creating the JSFunction, etc). But this alone isn't compelling enough to determine the default of how to organize APIs.

Overall, I'd prefer the TAG position on how to organize APIs to remain neutral. Some APIs may have material benefit from an "all static" object, but I don't think there's a clear default.

@leobalter
Copy link

Overall, I'd prefer the TAG position on how to organize APIs to remain neutral. Some APIs may have material benefit from an "all static" object, but I don't think there's a clear default.

I second this as it seems good as a case by case situation.

There is a nitpick here where examples with foo.bar vs fooBar are missing an important context: these names are not meaningful. As a counterpoint: showOpenFilePicker seem clear enough for me to not need a namespace.

For my personal taste, the names are just too long. That's countered by encouraging and taking advantage of code completion when available (dev tools, IDEs). This practice may be applied to globals and namespaced functions anyway.

The other important value for names should be the discoverability + learnability. If you give me something non generic such as showOpenFilePicker, I'm able to quickly find it on MDN, Google, and others. The namespace comes into play when methods might seem more generic and object specific, i.e.: Temporal and Intl.

@LeaVerou
Copy link
Member Author

LeaVerou commented Jun 5, 2023

As a related point, something that came up in #11 (or the f2f discussions around it) was classes that are not of general utility but are specific to certain APIs. E.g. there's a GeolocationCoordinates class that is related to the Geolocation API, shouldn't it have been Geolocation.Coordinates (since there's already a Geolocation object)? Some of the arguments against defining data-centric classes that were mentioned in the discussions related to #11 were exactly around this: that we don't want this kind of pollution. However, if relevant classes are namespaced under the main object of the API they belong to this reduces that issue (and improves discoverability), without really making anything more verbose.

@annevk
Copy link
Member

annevk commented Jun 6, 2023

If we did that API today it would be a dictionary. It's also not really clear to me how adding a dot improves things, as the pollution is not about the name, it's about using a class where a dictionary suffices. If it actually needed a class it'd have been fine.

@ljharb
Copy link

ljharb commented Jun 6, 2023

It's still nice to have fewer globals and more context about the scope of a thing.

@bathos
Copy link

bathos commented Jun 6, 2023

There are currently no web platform constructors with static properties pointing at other constructors like Geolocation.Coordinates. Web IDL can’t express that pattern currently. ES doesn’t use that pattern, either. It seems like the bar should be pretty high for introducing a new class organization / discovery pattern on the web platform, whose graveyard for such things is probably at capacity.

If people were to conclude something like that is desirable to encourage, though (or to stop discouraging), I think de-legacying the existing [LegacyNamespace] extended attribute would achieve the same organizational effect without adding yet-another-way-things-are-done to the mix. That extended attribute causes interface objects to be exposed via data properties of Web IDL namespace objects (i.e., non-callable ordinary objects — not other constructors) instead of the global interface object directly. The WebAssembly namespace is a good example of its usage.

Notably, ECMA-262 & 402 employ what’s effectively the same pattern (Intl & Temporal) and afaict it doesn’t look as though TC39 regards it as “legacy” (considering Temporal is new).

@LeaVerou
Copy link
Member Author

LeaVerou commented Mar 5, 2024

@domenic

  • foo.bar() is not really better than fooBar(); shared prefixes without a dot are just as good as shared prefixes with a dot.

The problem with fooBar() is that it's not really clear which part of the name is a namespace and what is specific to the function. foo.bar() communicates strictly more information than fooBar().

  • This encourages the proliferation of non-constructible singleton classes, which is against other design principles: 1, 2.

This principle is about objects for which you can meaningfully have multiple instances. It explicitly does not discourage namespaces, for which Web IDL has a separate feature.

  • TC39's design choices in this space are not a good precedent; their duplicative functions cause developer confusion for no real benefit, and their namespacing of things (e.g. under Intl or Temporal) is actively in conflict with how we do things on the web (e.g., we didn't namespace DOM.Node, DOM.Element, DOM.Attr).

I do think patterns like Intl or Temporal are more of a gray area than the examples I listed in my first post. And indeed, nobody would want DOM.Node — we definitely wouldn't want to turn the web platform into Java! That said, something like HTML.Element, HTML.ParagraphElement etc wouldn't be unreasonable, since HTML is already effectively a namespace by convention, so why not get the benefits of an actual namespace?

Anyhow, it seems obvious that there's a point where namespacing can be taken too far, it just seems that different people/groups have differing opinions about what that point is. Perhaps we can find what is the lowest common denominator where we have consensus and form guidance around that? Remember that all design principles are rules of thumb, it doesn't mean that if we have a design principle discouraging a pattern, we can never do that, just that we need to make sure we're doing it for good reason. They are meant as guidance, not orders.

For example, it occurred to me recently that fetch() is an excellent example of a function that is global and I think we'd probably all agree that was a great decision. I wonder if we can figure out what the pattern is.

  • The equivalence between web platform functions and author functions is false. The global namespace is the web platform's territory; ever since the failure of built-in modules, this has become even more clear. Author code belongs in modules, but the web platform's standard library lives on the global object. This is true whether or not those standard library functions have a . in their name, or not.

We don't discourage authors from adding a bunch of globals so they can avoid collisions with the web platform, but also between each other. The same thing applies to the web platform.
There is a tremendous number of people designing APIs for the web platform these days, so collisions are not that unlikely. We've already had several cases where we had to make different naming choices because of collisions with existing technologies.
The point about namespaces is that they reduce collisions by making the naming structure hierarchical, rather than flat. Meaning that obviously having Foo.bar is not necessarily less likely to collide with another name than FooBar, but if you have both Foo.bar and Foo.baz you're still only allocating one global, whereas FooBar and FooBaz would require two.

  • Guidance like this often leads people to putting their functions under document or navigator, when they have nothing to do with Documents or with user agent data. I've personally had to advise several teams to move away from such designs in their proposals. (navigator is particularly polluted these days, unfortunately. A design principle discouraging such pollution, and encouraging using the global object instead, would be great!)

That is a red herring — if the actual problem is people adding functions on document or navigator then we should have a principle providing guidance on that.
In fact, @jyasskin opened a separate issue on this: #448

However, none of the examples in the first post fall under that category. The Filesystem API involves enough methods that it could (and should) have added its own namespace, and for the others there were already objects that should have had these methods (as static methods).

  • The idea of reserving a namespace "just in case" you need it later in an API expansion often leads to bad APIs, e.g. this week I've had to advise a team to avoid window.tokenAttestation.getStateToken() (no other methods) in favor of window.getAttestationStateToken().

It's a balance, we also wouldn't want a proliferation of namespaces with only one function. I would encourage opening discussions in https://github.com/w3ctag/design-reviews/discussions/categories/q-a for these kinds of hairy API design issues, that's what that section is for :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Agenda+ tc39-tracker Issues where TC39 input would be useful Topic: JS
Projects
None yet
Development

No branches or pull requests

8 participants