loading frontend code as modules

[James Long]

We talked a good bit about this on IRC this morning, but it's probably more
appropriate here so everyone can see it. I don't know how to structure this
any better, sorry for the dump of text.

I really want to use modules on the frontend. It's not just about avoiding
having to order script tags correctly. It makes it a lot easier to see how
the system works by having the dependencies of each module live in the
module itself. It also encourages lots of smaller modules.

The perf tool already uses modules to some extent by reusing the devtools
loader. This is great. It's a good way to have a more modular structure
already.

But while it's a good immediate solution, there's a caveat: modules are not
in browser scope. You have to pass `window` in, which arguably is actually
better. But still, we want our tools to be more like normal webpages. There
are a lot of wins in that.

The devtools loader is a static instance that lives forever once it's
created. It loads modules once and they exist in cache forever. This is
also not like a webpage, and makes it impossible to somehow inject the
window into the global scope (since the module instance cannot reference
window at all, one module instance serves all the windows forever).

The devtools require is perfectly fine for system modules. All I want to do
is change this:

<script src="foo.js"></script>
<script src="bar.js"></script>

To this:

const foo = require('foo');
const bar = require('bar');

Using script tags, files are already loaded fresh each time the tool is
created. It's fine to do that for files specific to the tool, and that's
how a normal webpage would work, which is great.

We can create a loader that loads modules with `window` as the global scope
easily:

Loader({
id: "jsdebugger",
paths: loaderOptions.paths,
globals: loaderOptions.globals,
sharedGlobal: true,
sandboxPrototype: window,
})

This works great. Except that now any system/SDK modules (like promise) is
loaded as a fresh instance, while we want to use the devools loader cache
for system modules.

Basically: we want to just load our local files dynamically every time, but
load system files from the devtools loader. I don't know the right solution
yet, but I'm sure you all have opinions.

[Nick Fitzgerald]

On Mon, Jul 6, 2015 at 10:51 AM, James Long <jl...@mozilla.com> wrote:

> But while it's a good immediate solution, there's a caveat: modules are not
> in browser scope. You have to pass `window` in, which arguably is actually
> better. But still, we want our tools to be more like normal webpages. There
> are a lot of wins in that.
>

​I think it is best to pass these things in explicitly. That way we don't
load duplicates of modules that almost everything loads, such as
DevToolsUtils, and it is clear what a modules dependencies are rather than
relying on implicit global scope.

[James Long]

On Mon, Jul 6, 2015 at 2:08 PM, Nick Fitzgerald <nfitz...@mozilla.com>
wrote:

Things like `DebToolsUtils` are not by default part of `window`. In an
ideal world, when everything on the frontend is in modules, there would be
nothing on `window`. You would still have to explicitly import
`DevToolsUtils` and any other system libs you need.

But things like `setTimeout`, or any other DOM API that we are used to
having in the browser, will be available. This opens up opportunities to
use npm packages, and be a lot more like a normal webpage.

[Victor Porof]

I second Nick.

Although familiar to many webdevs, I strongly believe we should stay clear of implicit globals, especially `window`. Passing them explicitly in constructors, functions, modules (whatever) is a mild inconvenience compared to having a mechanism that prevents us from leaking stuff between files, either accidentally or because we’re lazy. Furthermore, having objects implicitly imported because they share the same scope introduces a kind of code complexity that is hard to avoid even when having “good habits” policies.

In the perf tool almost everything is a small module, but a couple of small generic “controller” files creeped into the markup as script tags, because of laziness. This isn’t ideal and we’d like to change that (could be a good first bug).

Add-on SDK devs are already very familiar with this mostly globalless module system, where everything is required and if you need `window`, you pass in window. Adhering to this format would be best.

As a side-note, things like set/clearTimeout are available from Timers.jsm, which can easily be required. I’m not convinced webdevs would really be confused by this.

Victor

> _______________________________________________
> dev-developer-tools mailing list
> dev-devel...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-developer-tools

[Nick Fitzgerald]

On Mon, Jul 6, 2015 at 11:22 AM, James Long <jl...@mozilla.com> wrote:

>
> On Mon, Jul 6, 2015 at 2:08 PM, Nick Fitzgerald <nfitz...@mozilla.com>
> wrote:
>
>>
>>
>> On Mon, Jul 6, 2015 at 10:51 AM, James Long <jl...@mozilla.com> wrote:
>>
>>> But while it's a good immediate solution, there's a caveat: modules are
>>> not
>>> in browser scope. You have to pass `window` in, which arguably is
>>> actually
>>> better. But still, we want our tools to be more like normal webpages.
>>> There
>>> are a lot of wins in that.
>>>
>>
>>
>> ​I think it is best to pass these things in explicitly. That way we don't
>> load duplicates of modules that almost everything loads, such as
>> DevToolsUtils, and it is clear what a modules dependencies are rather than
>> relying on implicit global scope.
>>
>>
> Things like `DebToolsUtils` are not by default part of `window`.
>

I'm not saying that DevToolsUtils should be or would be exposed on the
global.

I'm saying that a *ton* of our code imports it (most?) and if we have
different loaders, then we will​

​instantiate the module once per loader rather than once altogether.​ This
is highly undesirable from a memory POV.

[James Long]

On Mon, Jul 6, 2015 at 2:39 PM, Nick Fitzgerald <nfitz...@mozilla.com>
wrote:

>
>
>

> I'm saying that a *ton* of our code imports it (most?) and if we have
> different loaders, then we will​
>
> ​instantiate the module once per loader rather than once altogether.​ This
> is highly undesirable from a memory POV.
>
>

You missed what I was saying (or maybe I wasn't clear). All modules outside
of the tool-specific files would act exactly the same as they do now. I
just want to convert script tags to require statements, that's all. System
modules would be loaded once and shared across everything.

[James Long]

On Mon, Jul 6, 2015 at 2:29 PM, Victor Porof <vpo...@mozilla.com> wrote:

> I second Nick.
>
> Although familiar to many webdevs, I strongly believe we should stay clear
> of implicit globals, especially `window`. Passing them explicitly in
> constructors, functions, modules (whatever) is a mild inconvenience
> compared to having a mechanism that prevents us from leaking stuff between
> files, either accidentally or because we’re lazy. Furthermore, having
> objects implicitly imported because they share the same scope introduces a
> kind of code complexity that is hard to avoid even when having “good
> habits” policies.
>

Personally I think it's rather easy to enforce this constraint because
eslint will error if you assign to a var not in scope (accidental global),
and so the only way to leak is if you explicitly do `window.foo = 5`, which
is rather explicit and doesn't really happen by default. Webdevs have
strong, simple codebases because of this even though we're stuck with a
global window.

It's not just mild convenience; it would open up the possibility to use npm
packages!

>
> Add-on SDK devs are already very familiar with this mostly globalless
> module system, where everything is required and if you need `window`, you
> pass in window. Adhering to this format would be best.
>

According to Matteo, they actually do the same sandboxPrototype trick to
expose window as a global to addons, for all the reasons we want to do it.
(I'm not too familiar with this, maybe it's just for some addons?)

[ZER0]

On Mon, Jul 6, 2015 at 8:48 PM, James Long <jl...@mozilla.com> wrote:

> It's not just mild convenience; it would open up the possibility to use npm
> packages!

I agree with you; it's important to notice that some packages won't be
easy to use due the fact that we will have both a "node like"
environment AND a browser one. Take react, for example: You can use
npm to install it and use it, but you have to require the browser
distribution – `require("react/dist/react")` – 'cause if you just do
`require("react")` it won't work – as far as I noticed, react, like
other modules, tries to understand in which context is loaded, and
having a hybrid context like ours doesn't make the thing easy for him.

>> Add-on SDK devs are already very familiar with this mostly globalless
>> module system, where everything is required and if you need `window`, you
>> pass in window. Adhering to this format would be best.

> According to Matteo, they actually do the same sandboxPrototype trick to
> expose window as a global to addons, for all the reasons we want to do it.
> (I'm not too familiar with this, maybe it's just for some addons?)

I was talking about content scripts, that are not part of main add-on
code, but are basically script injected in content process: they're
sandboxed, but because they have to behave like regular page's script,
they have to use the sandboxPrototype trick. However, they cannot
access to SDK module and they don't have `require`.

[J. Ryan Stinnett]

On Mon, Jul 6, 2015 at 1:48 PM, James Long <jl...@mozilla.com> wrote:
> On Mon, Jul 6, 2015 at 2:29 PM, Victor Porof <vpo...@mozilla.com> wrote:
>
>> I second Nick.
>>
>> Although familiar to many webdevs, I strongly believe we should stay clear
>> of implicit globals, especially `window`. Passing them explicitly in
>> constructors, functions, modules (whatever) is a mild inconvenience
>> compared to having a mechanism that prevents us from leaking stuff between
>> files, either accidentally or because we’re lazy. Furthermore, having
>> objects implicitly imported because they share the same scope introduces a
>> kind of code complexity that is hard to avoid even when having “good
>> habits” policies.
>>
>
> Personally I think it's rather easy to enforce this constraint because
> eslint will error if you assign to a var not in scope (accidental global),
> and so the only way to leak is if you explicitly do `window.foo = 5`, which
> is rather explicit and doesn't really happen by default. Webdevs have
> strong, simple codebases because of this even though we're stuck with a
> global window.
>

> It's not just mild convenience; it would open up the possibility to use npm
> packages!

We clarified on IRC that npm would only need window the packages that
target browsers specifically. General algorithm packages (that work on
client or server) only need things like setTimeout for globals[1]. Of
course, there's also the built-in SDK which would need emulating like
Jetpack started to do or like browserify has done.

>>
>> Add-on SDK devs are already very familiar with this mostly globalless
>> module system, where everything is required and if you need `window`, you
>> pass in window. Adhering to this format would be best.
>>
>
> According to Matteo, they actually do the same sandboxPrototype trick to
> expose window as a global to addons, for all the reasons we want to do it.
> (I'm not too familiar with this, maybe it's just for some addons?)

I am curious to learn more about this. Matteo, maybe you can link to
where this happens?

I only saw one use of sandboxPrototype[2], but it did not appear to
expose window. Also, that's not the bootstrap.js file used by JPM
addons. When I look at the JPM bootstrap[3], I did not see any
sandboxPrototype.

- Ryan

[1]: https://iojs.org/api/globals.html
[2]: https://dxr.mozilla.org/mozilla-central/source/addon-sdk/source/app-extension/bootstrap.js#279
[3]: https://dxr.mozilla.org/mozilla-central/source/addon-sdk/source/lib/sdk/addon/bootstrap.js

[Jordan Santell]

What's the benefit of having a window global in these modules? It sounds
like yet-another-loader-caveat where X is accessible in environment Y but
not Z. We'd then have some modules that handle a global window, and others
that take a passed in arg (all graph modules just take a doc or element).

Also, this will just lead to doing things in the global window, whether its
someone unfamiliar with a tool, a contributor, or a quick fix needed, where
the global starts mutating, or worse, views calling methods on other views.
eslint would help, but its not in all tools yet, we would get an error for
using a global, or ignore it, either way it doesn't sound like it prevents
that case. Not sharing globals just hard prevents things like that
happening.

The SDK does not expose a global window, but if you wanted to access
'window.performance' for example, you'd have to use a browser window, async
communicate with a content script, or explicitly define the global in the
loader.

Sharing a global window is web-by, but just seems like a footgun.

[James Long]

Ok, I might not be explaining myself very well, but I'll try...

On Mon, Jul 6, 2015 at 3:49 PM, Jordan Santell <jsan...@mozilla.com> wrote:

> What's the benefit of having a window global in these modules? It sounds
> like yet-another-loader-caveat where X is accessible in environment Y but
> not Z. We'd then have some modules that handle a global window, and others
> that take a passed in arg (all graph modules just take a doc or element).
>

I don't care specifically about `window` itself. Having all normal browser
APIs is what's important. This *removes* a loader caveat, where you are
doing web stuff but suddenly you don't have access to a web API that you
are used to. I thought we wanted to move our tools more like being normal
webpages.

Think long-term. When ES6 gets modules, surely we're going to end up using
that. And all those modules are going to be normal browser modules, not
some weird isolated from the browser module.

If anything, this is super important for migration. The majority of our
tools are stuck in a everything-is-global context and we've got to
modularize them, and having normal browser scope will hugely help a slow
migration path.

> Also, this will just lead to doing things in the global window, whether
> its someone unfamiliar with a tool, a contributor, or a quick fix needed,
> where the global starts mutating, or worse, views calling methods on other
> views. eslint would help, but its not in all tools yet, we would get an
> error for using a global, or ignore it, either way it doesn't sound like it
> prevents that case. Not sharing globals just hard prevents things like that
> happening.
>

I don't get this. There is *not* a shared global. A `var` decl at the top
is not global, it's all module scope. You would have to explicitly set
`window.foo = 5` or `setTimeout.bar = 10` or mutate an explicit global to
have side effects. There is no global scope, it's still all local by
default.

We just need a normal browser context.

> The SDK does not expose a global window, but if you wanted to access
> 'window.performance' for example, you'd have to use a browser window, async
> communicate with a content script, or explicitly define the global in the
> loader.
>

Yeah, I was wrong about the SDK.

[Victor Porof]

> What's the benefit of having a window global in these modules? It sounds like yet-another-loader-caveat where X is accessible in environment Y but not Z. We'd then have some modules that handle a global window, and others that take a passed in arg (all graph modules just take a doc or element).
>
> I don't care specifically about `window` itself. Having all normal browser APIs is what's important. This *removes* a loader caveat, where you are doing web stuff but suddenly you don't have access to a web API that you are used to.

I still don’t see the benefit either. Taking set/clearTimeout as an example again, that won’t be accessible even if you automatically import `window` in these modules. A webdev would just write `setTimeout`, not `window.setTimeout`, and only the latter would work with your proposal. There is no measurable difference between explaining that "one needs to require Timers.js", vs. “use window.setTimeout instead of just setTimeout”.

If anything, the above might lead to a greater source of confusion. Webdevs are used to `window` being a global *and* the context in which their code is running, so things like `setTimeout` or whatever other global (Xhr, Image, etc.) are accessible without going via `window`.

So how far should we go? Import more browser APIs as globals then, so that they’re directly accessible instead of going via window.foo? Which ones? When do we stop? How do we explain that some APIs are available as globals and others need to be imported, or accessible via window?

It just seems like we’re shooting ourselves in the foot, and it’s much easier to just say “the only rule is: if you need something, require it”.

Victor

[James Long]

On Mon, Jul 6, 2015 at 5:16 PM, Victor Porof <vpo...@mozilla.com> wrote:

>
> What's the benefit of having a window global in these modules? It sounds
>> like yet-another-loader-caveat where X is accessible in environment Y but
>> not Z. We'd then have some modules that handle a global window, and others
>> that take a passed in arg (all graph modules just take a doc or element).
>>
> I don't care specifically about `window` itself. Having all normal browser
> APIs is what's important. This *removes* a loader caveat, where you are
> doing web stuff but suddenly you don't have access to a web API that you
> are used to.
>
>
> I still don’t see the benefit either. Taking set/clearTimeout as an
> example again, that won’t be accessible even if you automatically import
> `window` in these modules. A webdev would just write `setTimeout`, not
> `window.setTimeout`, and only the latter would work with your proposal.
> There is no measurable difference between explaining that "one needs to
> require Timers.js", vs. “use window.setTimeout instead of just setTimeout”.
>
>

Argh, it seems not many people are not understanding what I am saying.
Let's take this script tag:

<script src="foo.js"></script>

We want to literally turn it into this:

const foo = require("foo");

The `foo` module is evaluated in the same place. For all the reasons I
mentioned in my reply to Jordan, this is good. If anything for migrating
everything we already have. You would just type `setTimeout`. The global
object *is* the window. It's exactly the same as the browser. Just like a
normal webpage...

Please read my posts above again? I don't know how else to explain it. I
can make a prototype and show you what I mean. We need to this if we hope
to migrate old tools.

[Jordan Santell]

Didn't consider the Web APIs; that's a really good use case for most
stateless APIs (timers, performance) and let's us not have to reinvent the
wheel.

We can also make a module that exposes web APIs from another window, where
context doesn't matter:

`let { performance, setTimeout } = require("window")`

That way we still have modules without some in between world where there is
both a window and an exports object. If we push hard on only using web APIs
rather than mutating the window object, I think the global sounds good; but
then why use modules? Seems like an either-or thing, or otherwise it'll be
a strange environment, not familiar to any dev.

On Jul 6, 2015 2:16 PM, "Victor Porof" <vpo...@mozilla.com> wrote:

>
> What's the benefit of having a window global in these modules? It sounds
>> like yet-another-loader-caveat where X is accessible in environment Y but
>> not Z. We'd then have some modules that handle a global window, and others
>> that take a passed in arg (all graph modules just take a doc or element).
>>
> I don't care specifically about `window` itself. Having all normal browser
> APIs is what's important. This *removes* a loader caveat, where you are
> doing web stuff but suddenly you don't have access to a web API that you
> are used to.
>
>
> I still don’t see the benefit either. Taking set/clearTimeout as an
> example again, that won’t be accessible even if you automatically import
> `window` in these modules. A webdev would just write `setTimeout`, not
> `window.setTimeout`, and only the latter would work with your proposal.
> There is no measurable difference between explaining that "one needs to
> require Timers.js", vs. “use window.setTimeout instead of just setTimeout”.
>

[Jordan Santell]

I take it back, it'd pretty much be browserify, with a global window that
has a require function; which is nice.

[James Long]

On Mon, Jul 6, 2015 at 5:28 PM, Jordan Santell <jsan...@mozilla.com> wrote:

> I take it back, it'd pretty much be browserify, with a global window that
> has a require function; which is nice.
>

Yes, this exactly. (could always use browserify, but sourcemaps are kinda a
pain...)