ClojureScript

simplify :none script inclusion if :main supplied

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code

Description

If :main namespace supplied - under :none optimization settings :output-to file should goog.require it. This would also allow script inclusion to be unified regardless of the level of optimization supplied, i.e.:

<script type="text/javascript" src="foo.js"></script>

Instead of

<script type="text/javascript" src="out/goog/base.js"></script>
<script type="text/javascript" src="foo.js"></script>
<script type="text/javascript">goog.require("foo.core");</script>

Activity

Hide
David Nolen added a comment -

This does mean concatenating the contents of goog.base into the deps file.

Show
David Nolen added a comment - This does mean concatenating the contents of goog.base into the deps file.
Hide
Thomas Heller added a comment -

Just a quick Note: Not sure what your plan is for :main but it in my experience it is important to make it a collection. A larger app quickly gains multiple entry points.

Show
Thomas Heller added a comment - Just a quick Note: Not sure what your plan is for :main but it in my experience it is important to make it a collection. A larger app quickly gains multiple entry points.
Hide
David Nolen added a comment -

I don't see the point of making it a collection, users can simply load other entry points themselves via the "main" namespace.

Show
David Nolen added a comment - I don't see the point of making it a collection, users can simply load other entry points themselves via the "main" namespace.
Hide
Andrew Rosa added a comment -

Is someone already working on this issue? I will familiarize with the compiler code to try tackle this feature.

Show
Andrew Rosa added a comment - Is someone already working on this issue? I will familiarize with the compiler code to try tackle this feature.
Hide
Andrew Rosa added a comment -

@dnolen I made the changes to compiler for auto require an specified :main namespace (but still optional). After that I started to experiment adding the goog.base code directly into the generated js, just to see what happens.

What I found is that we will need to disable the Closure default deps.js file. So we need to copy the goog.base code and after that include all dependencies, including the Closure ones. This work is need to avoid two issues:

  • We need to explicit set the base path for loading, since the auto-discovery method used by Closure searches for a <script str="base.js">.
  • By default, Closure automatically includes a <script> containing the goog/deps.js file to include itself. This causes race conditions during our require('main-ns'), since only half of our dependencies was added to dependency graph. Explicitly requiring everything will avoid this bug.

I could change the compiler to accommodate all this changes, but first like to communicate you the deep effects on our current behavior. What do you think?

Show
Andrew Rosa added a comment - @dnolen I made the changes to compiler for auto require an specified :main namespace (but still optional). After that I started to experiment adding the goog.base code directly into the generated js, just to see what happens. What I found is that we will need to disable the Closure default deps.js file. So we need to copy the goog.base code and after that include all dependencies, including the Closure ones. This work is need to avoid two issues:
  • We need to explicit set the base path for loading, since the auto-discovery method used by Closure searches for a <script str="base.js">.
  • By default, Closure automatically includes a <script> containing the goog/deps.js file to include itself. This causes race conditions during our require('main-ns'), since only half of our dependencies was added to dependency graph. Explicitly requiring everything will avoid this bug.
I could change the compiler to accommodate all this changes, but first like to communicate you the deep effects on our current behavior. What do you think?
Hide
Thomas Heller added a comment -

I have implemented this feature in shadow-build [1] and use the following strategy:

Assuming our output file is named "app.js" for an optimized build, it will look basically like this when compiling with :none.

1) imul.js fix
2) closure global settings

I found that these are required

var CLOSURE_NO_DEPS = true;
var CLOSURE_BASE_PATH = '/assets/js/src/';

The CLOSURE_BASE_PATH is required for the include scripts to find the javascripts for each namespace.

3) After the settings the goog/base.js is emitted into the file.
4) After that the contents of dependency settings are emitted (basically goog/deps.js) plus all of our namespaces. They look something like this

goog.addDependency("clojure/set.js",['clojure.set'],['cljs.core']);

When closure attempts to load a given namespace it will simply emit a <script src="CLOSURE_BASE_PATH + path/to/ns.js">, I found that using an absolute path is the best way to ensure that files are loaded correctly since the "path detection" feature of the goog/base.js file fails since we do not include a goog/base.js

5) Then finally you can just add the

goog.require('main-ns');

Hope that helps.

[1] https://github.com/thheller/shadow-build

Show
Thomas Heller added a comment - I have implemented this feature in shadow-build [1] and use the following strategy: Assuming our output file is named "app.js" for an optimized build, it will look basically like this when compiling with :none. 1) imul.js fix 2) closure global settings I found that these are required
var CLOSURE_NO_DEPS = true;
var CLOSURE_BASE_PATH = '/assets/js/src/';
The CLOSURE_BASE_PATH is required for the include scripts to find the javascripts for each namespace. 3) After the settings the goog/base.js is emitted into the file. 4) After that the contents of dependency settings are emitted (basically goog/deps.js) plus all of our namespaces. They look something like this
goog.addDependency("clojure/set.js",['clojure.set'],['cljs.core']);
When closure attempts to load a given namespace it will simply emit a <script src="CLOSURE_BASE_PATH + path/to/ns.js">, I found that using an absolute path is the best way to ensure that files are loaded correctly since the "path detection" feature of the goog/base.js file fails since we do not include a goog/base.js 5) Then finally you can just add the
goog.require('main-ns');
Hope that helps. [1] https://github.com/thheller/shadow-build
Hide
Andrew Rosa added a comment -

Thank you Thomas for your very nice writeup.

In my tests I found that we could leave CLOSURE_BASE_PATH alone IF we made everything relative to our output file. Do you think forcing it to some path is more reliable?

Show
Andrew Rosa added a comment - Thank you Thomas for your very nice writeup. In my tests I found that we could leave CLOSURE_BASE_PATH alone IF we made everything relative to our output file. Do you think forcing it to some path is more reliable?
Hide
Thomas Heller added a comment -

Yes definitly. The location of the javascript files usually is "fixed" once the app is configured. Either you put it locally at something like "/js" or something like a CDN, so you configure it once and forget about it. But thats my own personal opinion, I have no idea how other people are handling it. I had some problems with relative paths but can't remember exactly what they were only that CLOSURE_BASE_PATH fixed it.

Show
Thomas Heller added a comment - Yes definitly. The location of the javascript files usually is "fixed" once the app is configured. Either you put it locally at something like "/js" or something like a CDN, so you configure it once and forget about it. But thats my own personal opinion, I have no idea how other people are handling it. I had some problems with relative paths but can't remember exactly what they were only that CLOSURE_BASE_PATH fixed it.
Hide
Andrew Rosa added a comment -

Just to give an update here: I already have a patch for this issue, but depends on the resolution of CLJS-874.

Show
Andrew Rosa added a comment - Just to give an update here: I already have a patch for this issue, but depends on the resolution of CLJS-874.
Hide
Andrew Rosa added a comment - - edited

Here are the patches to fix this issue. I made smaller patches for each "step" to light the burden of code review.

  • 0001: this issue creates a new compiler option :main, which receives a symbol with the "main namespace" similar to Clojure. If supplied, will emit a require at the end of our custom deps file.
  • 0002: here I put the Closure configuration constants to make the loading works. I set the goog/base.js dir as a relative path so the change will be less intrusive to cljs compiler.
  • 0003: Emit whole goog.base
  • 0004: combine all deps into our single file, to avoid async dependency loading between google closure libs and our deps.
  • 0005: Update all sample documentation and remove the old "*-dev.html" files.

Thomas: thank you for the help and the examples. In the end I removed the need to know absolute paths by just considering we can aways reach the goog/base directory. This leads to less code involved to make the feature available.

Please let me know if there is any problem with those patches, so I could fix them. This is my first contribution to Clojure(Script), so I still need to grasp the workflow here.

Show
Andrew Rosa added a comment - - edited Here are the patches to fix this issue. I made smaller patches for each "step" to light the burden of code review.
  • 0001: this issue creates a new compiler option :main, which receives a symbol with the "main namespace" similar to Clojure. If supplied, will emit a require at the end of our custom deps file.
  • 0002: here I put the Closure configuration constants to make the loading works. I set the goog/base.js dir as a relative path so the change will be less intrusive to cljs compiler.
  • 0003: Emit whole goog.base
  • 0004: combine all deps into our single file, to avoid async dependency loading between google closure libs and our deps.
  • 0005: Update all sample documentation and remove the old "*-dev.html" files.
Thomas: thank you for the help and the examples. In the end I removed the need to know absolute paths by just considering we can aways reach the goog/base directory. This leads to less code involved to make the feature available. Please let me know if there is any problem with those patches, so I could fix them. This is my first contribution to Clojure(Script), so I still need to grasp the workflow here.
Hide
Martin Klepsch added a comment - - edited

@Andrew, I just applied your patches and got the Twitterbuzz example running.
You've done great work, lot's of people will appreciate this change.

One issue I encountered doing that:

In the Twitterbuzz Readme there's a statement as follows:

(def opts {:main 'twitterbuzz.core :output-to "samples/twitterbuzz/twitterbuzz.js" :output-dir "samples/twitterbuzz/out"})
[...]
The reason we set the `:output-dir` is because the `index.html` script tag is specifically pointing to that directory.

When compiling with these options index.html tries to load deps from "[...]clojurescript/samples/twitterbuzz/samples/twitterbuzz/out/twitterbuzz/dom_helpers.js" which is not correct.

After that I just compile with

(def opts {:main 'twitterbuzz.core :output-to "samples/twitterbuzz/twitterbuzz.js" :output-dir "out"})

This places an "out" dir into the clojurescript project directory. Now, when opening index.html, twitterbuzz.js looks for dependencies like this: "[...]clojurescript/samples/twitterbuzz/out/twitterbuzz/dom_helpers.js". This correctly loads the earlier compiled js and the application works. I'm not sure how this is supposed to work as I've mostly just used lein-cljsbuild to date but you probably have an idea

Some minor things

  • The referenced patch CLJS-874 doesn't apply cleanly anymore. I'm too unfamiliar working with patches to help with this unfortunately.
  • Is there any reason to specify :main in advanced compilation mode? (It has been added to the advanced mode section in the twitterbuzz example.)

Really looking forward to see this merged!

Show
Martin Klepsch added a comment - - edited @Andrew, I just applied your patches and got the Twitterbuzz example running. You've done great work, lot's of people will appreciate this change. One issue I encountered doing that: In the Twitterbuzz Readme there's a statement as follows:
(def opts {:main 'twitterbuzz.core :output-to "samples/twitterbuzz/twitterbuzz.js" :output-dir "samples/twitterbuzz/out"})
[...]
The reason we set the `:output-dir` is because the `index.html` script tag is specifically pointing to that directory.
When compiling with these options index.html tries to load deps from "[...]clojurescript/samples/twitterbuzz/samples/twitterbuzz/out/twitterbuzz/dom_helpers.js" which is not correct. After that I just compile with
(def opts {:main 'twitterbuzz.core :output-to "samples/twitterbuzz/twitterbuzz.js" :output-dir "out"})
This places an "out" dir into the clojurescript project directory. Now, when opening index.html, twitterbuzz.js looks for dependencies like this: "[...]clojurescript/samples/twitterbuzz/out/twitterbuzz/dom_helpers.js". This correctly loads the earlier compiled js and the application works. I'm not sure how this is supposed to work as I've mostly just used lein-cljsbuild to date but you probably have an idea Some minor things
  • The referenced patch CLJS-874 doesn't apply cleanly anymore. I'm too unfamiliar working with patches to help with this unfortunately.
  • Is there any reason to specify :main in advanced compilation mode? (It has been added to the advanced mode section in the twitterbuzz example.)
Really looking forward to see this merged!
Hide
Andrew Rosa added a comment -

Thanks @Martin for testing the patches, I kindly appreciate that.

I need to take some time to investigate the issue with this specific example. I'm not confident with the behavior of some examples, like the hello-js one which looks like broken (even on master). Probably I will provide a patch to review and possibly update the examples in another ticket, so we can have a more understandable environment to test this patches.

About the patch CLJS-874, it was made prior the last rework I've made here and probably will not be needed anymore. I need to confess that, if I knew at the time how to do multiple patches I neither need to created it in the first place.

At the last, the :main option isn't really used in advanced compilation. I just put it there to have some sort of "consistency" across all the examples - the only thing that change between them will be the optimization level. Do you think that this generates more confusion than good?

Show
Andrew Rosa added a comment - Thanks @Martin for testing the patches, I kindly appreciate that. I need to take some time to investigate the issue with this specific example. I'm not confident with the behavior of some examples, like the hello-js one which looks like broken (even on master). Probably I will provide a patch to review and possibly update the examples in another ticket, so we can have a more understandable environment to test this patches. About the patch CLJS-874, it was made prior the last rework I've made here and probably will not be needed anymore. I need to confess that, if I knew at the time how to do multiple patches I neither need to created it in the first place. At the last, the :main option isn't really used in advanced compilation. I just put it there to have some sort of "consistency" across all the examples - the only thing that change between them will be the optimization level. Do you think that this generates more confusion than good?
Hide
Mike Thompson added a comment - - edited

I love this idea. Ironing out these sorts of stumbling blocks is so important to achieving wider cljs adoption. When I was starting with CLJS at the beginning of the year, I accumulated a healthy set of paper cuts from this sort of stuff. WAY too much time was wasted.

But ... two things:

1. I'd be concerned that the concatenation of base.js and deps.js might slow things down. The lovely thing about `:none` currently is that it delivers sub-second compile times. That's so important to a good productive workflow. But perhaps I'm being paranoid and adding in a base.js pre-pend won't add much time.

2. You really don't need to know what :main is. You can take a sledgehammer to the process by appending the following to the generated file:

```
for(var namespace in goog.dependencies_.nameToPath)
goog.require(namespace);
```

This requires everything in. Everything. (Which is the same outcome as using :whitespace or :simple) And that turns out to be essential in a unit-testing scenario where there isn't a single root namespace. Further explanation:

https://github.com/mike-thompson-day8/cljsbuild-none-test-seed/blob/master/test.html#L110-L111

Aside: I worry that this might not be backwards compatible and that it should be achieved by introducing a new `:optimization` target of `:debug` (and deprecating :none ??). Just a thought.

Show
Mike Thompson added a comment - - edited I love this idea. Ironing out these sorts of stumbling blocks is so important to achieving wider cljs adoption. When I was starting with CLJS at the beginning of the year, I accumulated a healthy set of paper cuts from this sort of stuff. WAY too much time was wasted. But ... two things: 1. I'd be concerned that the concatenation of base.js and deps.js might slow things down. The lovely thing about `:none` currently is that it delivers sub-second compile times. That's so important to a good productive workflow. But perhaps I'm being paranoid and adding in a base.js pre-pend won't add much time. 2. You really don't need to know what :main is. You can take a sledgehammer to the process by appending the following to the generated file: ``` for(var namespace in goog.dependencies_.nameToPath) goog.require(namespace); ``` This requires everything in. Everything. (Which is the same outcome as using :whitespace or :simple) And that turns out to be essential in a unit-testing scenario where there isn't a single root namespace. Further explanation: https://github.com/mike-thompson-day8/cljsbuild-none-test-seed/blob/master/test.html#L110-L111 Aside: I worry that this might not be backwards compatible and that it should be achieved by introducing a new `:optimization` target of `:debug` (and deprecating :none ??). Just a thought.
Hide
Thomas Heller added a comment -

@Mike Don't worry, prepending base+deps.js is not noticeable performance wise (less than a millisecond).

I tried defining what a "main" is for quite some time in shadow-build but the best I came up with: It's a namespace that contains functions called from outside your CLJS Code (eg. via HTML, aka API functions). It is a very valueable thing to have since its the only way to tell the CLJS compiler (and runtime) which namespaces are actually required. If you have a "main" namespace its deps (and the deps of the deps, ...) can be identified and namespaces that aren't actually needed can be skipped. The Closure dead-code elimination does this for :advanced as well, we just need a little help for :none.

IMHO every app should have at least one "main" ns, requiring everything is a hack not a solution.

As for your clojurescript.test example, that is a shortcoming of the tools.

I'm happy to provide help with shadow-build if anyone is interested, should be pretty simple to add a :test target. To be honest adding more features/options to cljs.closure/build (which lein-cljsbuild uses) is a recipe for disaster as it is already way too complex.

Show
Thomas Heller added a comment - @Mike Don't worry, prepending base+deps.js is not noticeable performance wise (less than a millisecond). I tried defining what a "main" is for quite some time in shadow-build but the best I came up with: It's a namespace that contains functions called from outside your CLJS Code (eg. via HTML, aka API functions). It is a very valueable thing to have since its the only way to tell the CLJS compiler (and runtime) which namespaces are actually required. If you have a "main" namespace its deps (and the deps of the deps, ...) can be identified and namespaces that aren't actually needed can be skipped. The Closure dead-code elimination does this for :advanced as well, we just need a little help for :none. IMHO every app should have at least one "main" ns, requiring everything is a hack not a solution. As for your clojurescript.test example, that is a shortcoming of the tools. I'm happy to provide help with shadow-build if anyone is interested, should be pretty simple to add a :test target. To be honest adding more features/options to cljs.closure/build (which lein-cljsbuild uses) is a recipe for disaster as it is already way too complex.
Hide
Andrew Rosa added a comment -

I'm also against introducing new options. I really want is eventually deprecate some of the unneeded ones.

I think that the :main namespace is clean, and could avoid issues like having some code evaluated in a namespace that is removed by Closure compiler in production. Not sure how common that could be in ClojureScript, but a valid buggy scenario in terms of global mutable state.

Anyway, if we follow the alternative of requiring everything, I think that could be done without dabbling with goog.base internals. We already have all app namespaces to be able to emit the deps.js

@Thomas your experience with shadow-build has shown you any need of "multiple entry-points" or only a single :main ns is enough? I have a feeling that the need to include all test namespaces is somewhat necessary because how clojurescript.test works, but I'm not familiar how the Clojure version works.

Show
Andrew Rosa added a comment - I'm also against introducing new options. I really want is eventually deprecate some of the unneeded ones. I think that the :main namespace is clean, and could avoid issues like having some code evaluated in a namespace that is removed by Closure compiler in production. Not sure how common that could be in ClojureScript, but a valid buggy scenario in terms of global mutable state. Anyway, if we follow the alternative of requiring everything, I think that could be done without dabbling with goog.base internals. We already have all app namespaces to be able to emit the deps.js @Thomas your experience with shadow-build has shown you any need of "multiple entry-points" or only a single :main ns is enough? I have a feeling that the need to include all test namespaces is somewhat necessary because how clojurescript.test works, but I'm not familiar how the Clojure version works.
Hide
Thomas Heller added a comment -

@Andrew: See my first comment on this issue, in my experience an app will have more than one entry point. "clojurescript.test" would be one example why.

Show
Thomas Heller added a comment - @Andrew: See my first comment on this issue, in my experience an app will have more than one entry point. "clojurescript.test" would be one example why.
Hide
Martin Klepsch added a comment -

@thomas could you further explain the behaviour you'd expect from passing multiple namespaces to :main?

With my current understanding I don't see a need for multiple arguments to the :main option as Andre proposed it.
If you want a build that has another entry point (which also loads tests or similar) why not just make it a different build?

Show
Martin Klepsch added a comment - @thomas could you further explain the behaviour you'd expect from passing multiple namespaces to :main? With my current understanding I don't see a need for multiple arguments to the :main option as Andre proposed it. If you want a build that has another entry point (which also loads tests or similar) why not just make it a different build?
Hide
Thomas Heller added a comment -

It's always rather difficult to explain something without a good real world example, but explaining what I do in my app is too app specific. Anyways, my web app has a couple of different features. Lots of different pages and some use one or more features provided by CLJS code. It is not a single-page Application, rather an "old" web app with just some dynamic components sprinkled in.

Let me try an example:

(ns my-app.feature1)
(defn ^:export fancy-component [])

(ns my-app.feature2)
(defn ^:export fancy-component [])

(ns my-app.feature3)
(defn ^:export fancy-component [])

These would all be "main" namespaces by my definition.

I do not want a (ns my-app (:require [my-app [feature1 feature2 feature3]])) since that would couple all components together. One feature provided by shadow-build is that it is able to "split" the produced javascript into multiple files (modules) so you may load them optionally. So I can even split each feature into a seperate .js file loadable on demand, AFTER advanced compilation that is.

I do this in my web app and instead of feeding everyone a hefty 800kb javascript file, everyone gets 90kb and loads the rest when needed. This greatly improved page load times.

The problem with multiple builds is that EVERY build will contain cljs.core, which is quite heavy. You'd end up with

build1: cljs.core, my-app.feature1
build2: cljs.core, my-app.feature2
build3: cljs.core, my-app.feature3

instead of

module1: cljs.core (+ code shared by feature1,2,3)
module2: my-app.feature1
module3: my-app.feature2
module4: my-app.feature3

If you have experience with jQuery, multiple builds would be the equivalent to including jQuery with every jQuery plugin. But I'm getting off topic, thats about modules not about :main.

Hope I made some sense in regards to multiple mains, its just something I use alot but for me it is coupled with modules so YMMV.

Show
Thomas Heller added a comment - It's always rather difficult to explain something without a good real world example, but explaining what I do in my app is too app specific. Anyways, my web app has a couple of different features. Lots of different pages and some use one or more features provided by CLJS code. It is not a single-page Application, rather an "old" web app with just some dynamic components sprinkled in. Let me try an example: (ns my-app.feature1) (defn ^:export fancy-component []) (ns my-app.feature2) (defn ^:export fancy-component []) (ns my-app.feature3) (defn ^:export fancy-component []) These would all be "main" namespaces by my definition. I do not want a (ns my-app (:require [my-app [feature1 feature2 feature3]])) since that would couple all components together. One feature provided by shadow-build is that it is able to "split" the produced javascript into multiple files (modules) so you may load them optionally. So I can even split each feature into a seperate .js file loadable on demand, AFTER advanced compilation that is. I do this in my web app and instead of feeding everyone a hefty 800kb javascript file, everyone gets 90kb and loads the rest when needed. This greatly improved page load times. The problem with multiple builds is that EVERY build will contain cljs.core, which is quite heavy. You'd end up with build1: cljs.core, my-app.feature1 build2: cljs.core, my-app.feature2 build3: cljs.core, my-app.feature3 instead of module1: cljs.core (+ code shared by feature1,2,3) module2: my-app.feature1 module3: my-app.feature2 module4: my-app.feature3 If you have experience with jQuery, multiple builds would be the equivalent to including jQuery with every jQuery plugin. But I'm getting off topic, thats about modules not about :main. Hope I made some sense in regards to multiple mains, its just something I use alot but for me it is coupled with modules so YMMV.
Hide
Mike Thompson added a comment -

First, I'm a bit surprised. I thought there would be joy at the thought of NOT having to nominate a :main.

At its core, CLJS-851 appears to be about harmonizing `:none` with other options. The initial comment above explains how the HTML files for :none have to be different from the other options :simple etc. We don't like that. BUT the proposal to add :main into project.clj seems set to reintroduce a difference again between :none and the other options, but just in a different place. And there seems to be no need for that difference. :simple and :whitespace bring in all the code. So does :optimized except it is first tree-shaken down by Google Closure. So these 3 options do not need a main. Why should :none have a :main when the other options don't. I can see no downside to pulling in everything and making :none harmonious with the other.

@Andrew I agree that we don't need to use the internals of goog to require everything in. The necessary list of namespaces to require are present at the time of writing deps, so a list of goog.requires could be added, one for each namespace compiled.

Second, @Thomas could you explain more why you think cemerick's framework is wrong to be rootless? In my experience when unittesting you actively do not want a root. You want a directory full of test clj files, with that set of files growing and shrinking over time. Different members of the team might be adding and removing them at each step. You want your tests to be "discovered" by your "test runner" and then executed. I don't want to have to be manually maintaining a central list of tests. In my experience, tools like "Nose" under Python come with a "runner" which discovers all the tests by walking the directory of your project, but that approach isn't an option in the CLJS world. Instead, the compiler has to scoop up all the tests from the directory, compile them and then they have to be "required in", ONLY at that point can the tests be "discovered" in a CLJS world. At least that's my impression.

Proposal: if `:main` isn't there, then bring it all it as per my suggestion. If :main is there then only it gets required.

Show
Mike Thompson added a comment - First, I'm a bit surprised. I thought there would be joy at the thought of NOT having to nominate a :main. At its core, CLJS-851 appears to be about harmonizing `:none` with other options. The initial comment above explains how the HTML files for :none have to be different from the other options :simple etc. We don't like that. BUT the proposal to add :main into project.clj seems set to reintroduce a difference again between :none and the other options, but just in a different place. And there seems to be no need for that difference. :simple and :whitespace bring in all the code. So does :optimized except it is first tree-shaken down by Google Closure. So these 3 options do not need a main. Why should :none have a :main when the other options don't. I can see no downside to pulling in everything and making :none harmonious with the other. @Andrew I agree that we don't need to use the internals of goog to require everything in. The necessary list of namespaces to require are present at the time of writing deps, so a list of goog.requires could be added, one for each namespace compiled. Second, @Thomas could you explain more why you think cemerick's framework is wrong to be rootless? In my experience when unittesting you actively do not want a root. You want a directory full of test clj files, with that set of files growing and shrinking over time. Different members of the team might be adding and removing them at each step. You want your tests to be "discovered" by your "test runner" and then executed. I don't want to have to be manually maintaining a central list of tests. In my experience, tools like "Nose" under Python come with a "runner" which discovers all the tests by walking the directory of your project, but that approach isn't an option in the CLJS world. Instead, the compiler has to scoop up all the tests from the directory, compile them and then they have to be "required in", ONLY at that point can the tests be "discovered" in a CLJS world. At least that's my impression. Proposal: if `:main` isn't there, then bring it all it as per my suggestion. If :main is there then only it gets required.
Hide
Thomas Heller added a comment -

@Mike: Without getting too much into the specifics of shadow-build: The feature of discovering tests and running them for you would be trivial, even without making a single change in shadow-build itself.

"... that approach isn't an option in the CLJS world" is simply not true. It is true for cljs.closure/build since its a function combining multiple very complex tasks into one simple map of options. lein-cljsbuild thereby suffers the same fate.

I haven't done much work documenting or promoting shadow-build and to be honest I'd much rather see tools like lein-cljsbuild built on top of shadow-build rather than trying to turn it into a lein plugin itself. Quite frankly to me it is perfect the way it is. I can step into every stage of the CLJS compilation/optimization and do stuff other tools simply cannot do, but that comes at the price of not being very beginner friendly. Documentation might help but I'm really bad at writing that stuff.

Anyways, I would inclue :main for EVERY build, ESPECIALLY :advanced!

I have a rather large codebase (way over 10k LOC CLJS) with several distinct builds. If I had to compile every CLJS file for every build everything would build forever. Some smaller builds just require a couple files (say 10), why feed 300+ into the Closure Compiler to eliminate what a simple :main could have done (and even skipped the CLJS->JS compilation).

I have this stuff in production for about 2 years now, but with custom tools for about as long. I cannot speak for the lein-cljsbuild world whether :main would be a good idea there or not. I don't even care since shadow-build is not affected by this change at all, but given my experience with my own app/codebase I would recommend making :main accept a collection of namespaces.

Show
Thomas Heller added a comment - @Mike: Without getting too much into the specifics of shadow-build: The feature of discovering tests and running them for you would be trivial, even without making a single change in shadow-build itself. "... that approach isn't an option in the CLJS world" is simply not true. It is true for cljs.closure/build since its a function combining multiple very complex tasks into one simple map of options. lein-cljsbuild thereby suffers the same fate. I haven't done much work documenting or promoting shadow-build and to be honest I'd much rather see tools like lein-cljsbuild built on top of shadow-build rather than trying to turn it into a lein plugin itself. Quite frankly to me it is perfect the way it is. I can step into every stage of the CLJS compilation/optimization and do stuff other tools simply cannot do, but that comes at the price of not being very beginner friendly. Documentation might help but I'm really bad at writing that stuff. Anyways, I would inclue :main for EVERY build, ESPECIALLY :advanced! I have a rather large codebase (way over 10k LOC CLJS) with several distinct builds. If I had to compile every CLJS file for every build everything would build forever. Some smaller builds just require a couple files (say 10), why feed 300+ into the Closure Compiler to eliminate what a simple :main could have done (and even skipped the CLJS->JS compilation). I have this stuff in production for about 2 years now, but with custom tools for about as long. I cannot speak for the lein-cljsbuild world whether :main would be a good idea there or not. I don't even care since shadow-build is not affected by this change at all, but given my experience with my own app/codebase I would recommend making :main accept a collection of namespaces.
Hide
Mike Thompson added a comment -

@Thomas I'm looking at this through the cljsbuild point of view. I appreciate that shadow build might be able to do test discovery, but unfortunately I don't have the option of using it. So I'm explaining this from the perspective of a cljsbuild user, who is seeking a better outcome.

Show
Mike Thompson added a comment - @Thomas I'm looking at this through the cljsbuild point of view. I appreciate that shadow build might be able to do test discovery, but unfortunately I don't have the option of using it. So I'm explaining this from the perspective of a cljsbuild user, who is seeking a better outcome.
Hide
Andrew Rosa added a comment -

Nice to see this conversation going forward @Mike and @Thomas.

Anyone here can confirm that the advanced compilation behaves like a concatenation of all user defined namespaces? If so, we could change our strategy to mimic the behavior on :none - consistency is a gold IMO, and I like the idea of transparency in optimization levels. Thanks @Mike to point it out.

@Thomas, I guess that the way you suggest to modularize the builds is not possible even for :advanced compilations. I prefer to have a separate issue to discuss and address that feature (which looks interesting for use cases like you presented). And sorry to make you answer the same question again.

The conditional :main is worth the cost of added complexity? How it will affect :advanced compilation?

Show
Andrew Rosa added a comment - Nice to see this conversation going forward @Mike and @Thomas. Anyone here can confirm that the advanced compilation behaves like a concatenation of all user defined namespaces? If so, we could change our strategy to mimic the behavior on :none - consistency is a gold IMO, and I like the idea of transparency in optimization levels. Thanks @Mike to point it out. @Thomas, I guess that the way you suggest to modularize the builds is not possible even for :advanced compilations. I prefer to have a separate issue to discuss and address that feature (which looks interesting for use cases like you presented). And sorry to make you answer the same question again. The conditional :main is worth the cost of added complexity? How it will affect :advanced compilation?
Hide
Thomas Heller added a comment -

Advanced Optimization does not behave like a concatenation, Closure handles the optimization like this (greatly simplified):

1) You provide a list of inputs (Javascript files) and externs
2) Closure parses these into AST inputs
3) basic analysis to extract goog.provide/require information
4) build dependency graph, topological sort
5) magic (if :advanced)
6) generate output

shadow-build does something similar

1) scan all source paths for "resources" (.js, .cljs)
2) extract goog.provide/require from .js
3) read first form of .cljs files (expects it to be 'ns), get provide/require from that
4) build dependency graph, topological sort
5) compile CLJS->JS (if reachable by any :main)
6) feed into Closure Compiler
7) output

Basically cljs.clojure/build does the same, except for #3 which results in CLJS compiling in a somewhat random order (eg. it compiles one 'ns and all its deps until everything is compiled). Without a :main everything will be compiled even if not used anywhere in your project. Closure Advanced Compilation can still remove dead-code since it will analyze the AST and realize that parts (or everything) of a given namespace are never used, :none can not do that without a :main.

If you have 300 source files (CLJS+JS) the load time of a web page (in dev) will be rather high (we are talking seconds here, remember worrying about a couple ms), especially with open dev tools since that will also load an additional 300 source maps. Loading files you don't need therefore is a huge waste of time especially if you reload (or even phantomjs clojurescript.test) often. I cannot stress enough how important :main is for this stuff, but it largely depends on the size of your codebase and the structure of your app. Small example: the Admin "App" of my application will do 714 requests on page load (JS+source maps) and transfer 3.3mb, page load is 3,54s on my rather beefy MacPro (in production this cuts down 13 requests, 343kb and page load in 700ms). Without the :main every page in my app would do 714 requests (or even more) on page load. Not a great development experience. Again I'm using my own tooling but do things like lein-figwheel not trigger full page reloads? REPL might provide a different experience but I found it impossible to properly build/test complex input forms when using a REPL, much rather edit/reload.

@Andrew: Agreed that the module discussion has no place here, just wanted to point out that the :main option becomes mandatory then. To be honest this whole discussion only stresses the problem of cljs.closure/build: trying to combine everyone's expectation of how a build should look into a single function is tough. So in the long run something that lets you split these parts will be easier (hint: shadow-build) but the good part is that ClojureScript itself does not even need to deal with all this stuff. A "simple" CLJS->JS compiler would do, the Closure bits are optional after all (even more so for non-browser targeted builds). But discussing what should be in CLJS and what shouldn't is another issue as well.

Show
Thomas Heller added a comment - Advanced Optimization does not behave like a concatenation, Closure handles the optimization like this (greatly simplified): 1) You provide a list of inputs (Javascript files) and externs 2) Closure parses these into AST inputs 3) basic analysis to extract goog.provide/require information 4) build dependency graph, topological sort 5) magic (if :advanced) 6) generate output shadow-build does something similar 1) scan all source paths for "resources" (.js, .cljs) 2) extract goog.provide/require from .js 3) read first form of .cljs files (expects it to be 'ns), get provide/require from that 4) build dependency graph, topological sort 5) compile CLJS->JS (if reachable by any :main) 6) feed into Closure Compiler 7) output Basically cljs.clojure/build does the same, except for #3 which results in CLJS compiling in a somewhat random order (eg. it compiles one 'ns and all its deps until everything is compiled). Without a :main everything will be compiled even if not used anywhere in your project. Closure Advanced Compilation can still remove dead-code since it will analyze the AST and realize that parts (or everything) of a given namespace are never used, :none can not do that without a :main. If you have 300 source files (CLJS+JS) the load time of a web page (in dev) will be rather high (we are talking seconds here, remember worrying about a couple ms), especially with open dev tools since that will also load an additional 300 source maps. Loading files you don't need therefore is a huge waste of time especially if you reload (or even phantomjs clojurescript.test) often. I cannot stress enough how important :main is for this stuff, but it largely depends on the size of your codebase and the structure of your app. Small example: the Admin "App" of my application will do 714 requests on page load (JS+source maps) and transfer 3.3mb, page load is 3,54s on my rather beefy MacPro (in production this cuts down 13 requests, 343kb and page load in 700ms). Without the :main every page in my app would do 714 requests (or even more) on page load. Not a great development experience. Again I'm using my own tooling but do things like lein-figwheel not trigger full page reloads? REPL might provide a different experience but I found it impossible to properly build/test complex input forms when using a REPL, much rather edit/reload. @Andrew: Agreed that the module discussion has no place here, just wanted to point out that the :main option becomes mandatory then. To be honest this whole discussion only stresses the problem of cljs.closure/build: trying to combine everyone's expectation of how a build should look into a single function is tough. So in the long run something that lets you split these parts will be easier (hint: shadow-build) but the good part is that ClojureScript itself does not even need to deal with all this stuff. A "simple" CLJS->JS compiler would do, the Closure bits are optional after all (even more so for non-browser targeted builds). But discussing what should be in CLJS and what shouldn't is another issue as well.
Hide
Martin Klepsch added a comment - - edited

Again I'm using my own tooling but do things like lein-figwheel not trigger full page reloads?

No. Figwheel only reloads changed files. So the slowdown should be a lot smaller once the page is loaded.

Besides that: @thomas thanks for all your very thoughtful messages here, just reading them already taught me a ton about Clojurescript compilation!

Show
Martin Klepsch added a comment - - edited
Again I'm using my own tooling but do things like lein-figwheel not trigger full page reloads?
No. Figwheel only reloads changed files. So the slowdown should be a lot smaller once the page is loaded. Besides that: @thomas thanks for all your very thoughtful messages here, just reading them already taught me a ton about Clojurescript compilation!
Hide
Andrew Rosa added a comment -

Thank you @Thomas for all this explanation, quite insightful. You convinced me that the way to go is requiring :main anyway. If we fell the necessity, we could add the "require all" behavior later. I also like the idea of having a dependency graph to compile only required stuff, but is something for another discussion

Just a final tweak for us to think about: the :main option could receive a list of namespaces or just a single one? Conceptually, I biased for the later. A single namespace could allow us to eventually add something like a -main function, useful for node apps for example - just an idea, of course.

I just updated the patches to fix a single issue with :main not being mungled before require, which caused problems with namespaces including dashes (like foo-bar)

Show
Andrew Rosa added a comment - Thank you @Thomas for all this explanation, quite insightful. You convinced me that the way to go is requiring :main anyway. If we fell the necessity, we could add the "require all" behavior later. I also like the idea of having a dependency graph to compile only required stuff, but is something for another discussion Just a final tweak for us to think about: the :main option could receive a list of namespaces or just a single one? Conceptually, I biased for the later. A single namespace could allow us to eventually add something like a -main function, useful for node apps for example - just an idea, of course. I just updated the patches to fix a single issue with :main not being mungled before require, which caused problems with namespaces including dashes (like foo-bar)
Hide
Mike Thompson added a comment -

Okay, my final observations:

  • naming
    Given :main is to be used to indicate a namespace, should its name be changed to make that clearer?
    After all this is about automating: 'goog.require(something)' but the name :main seems unrelated.
    To me :main carries connotations of main functions, not root namespaces.
    I would suggest that ":auto-require" would be a better name and I believe names (cumulatively) matter.
  • Special Marker
    No matter the name chosen, to handle my unittest use-case, would it be possible to have a
    special marker value for :main like, say, "*" which triggers the require everything approach.
    As I've indicated, that would be a big win from where I'm standing.

Thanks to everyone.

Show
Mike Thompson added a comment - Okay, my final observations:
  • naming Given :main is to be used to indicate a namespace, should its name be changed to make that clearer? After all this is about automating: 'goog.require(something)' but the name :main seems unrelated. To me :main carries connotations of main functions, not root namespaces. I would suggest that ":auto-require" would be a better name and I believe names (cumulatively) matter.
  • Special Marker No matter the name chosen, to handle my unittest use-case, would it be possible to have a special marker value for :main like, say, "*" which triggers the require everything approach. As I've indicated, that would be a big win from where I'm standing.
Thanks to everyone.
Hide
Andrew Rosa added a comment -

Good call @Mike. I guess :main was chosen to provide some "parity" with Clojure - that's why I suggest the auto-calling -main feature.

For the unit test case, do you think that it could be done as one enhancement on clojurescript.test? It seems like the test runner should be capable of finding the right namespaces... not sure if it does not do that for limitations of the platform.

Show
Andrew Rosa added a comment - Good call @Mike. I guess :main was chosen to provide some "parity" with Clojure - that's why I suggest the auto-calling -main feature. For the unit test case, do you think that it could be done as one enhancement on clojurescript.test? It seems like the test runner should be capable of finding the right namespaces... not sure if it does not do that for limitations of the platform.
Hide
Thomas Heller added a comment -

Closure calls them Entry Points, I got used to :main.

Can't really say much about clojurescript.test, only that testing generally is rather complex hiding this behind a "require all" marker will probably not be enough. I know I don't always want to run every single test, usually only those affected by the file I just changed is enough. But getting off-topic again, proper Tooling is hard.

Again for :main, my advice would be a collection but just offer a fallback and turn a single symbol into a collection of one. That covers all cases 0,1,more ... No :main just does not emit any requires which is what we have now.

:main my-app.feature1
:main [my-app.feature1]
:main [my-app.feature1 my-app.feature2]
Show
Thomas Heller added a comment - Closure calls them Entry Points, I got used to :main. Can't really say much about clojurescript.test, only that testing generally is rather complex hiding this behind a "require all" marker will probably not be enough. I know I don't always want to run every single test, usually only those affected by the file I just changed is enough. But getting off-topic again, proper Tooling is hard. Again for :main, my advice would be a collection but just offer a fallback and turn a single symbol into a collection of one. That covers all cases 0,1,more ... No :main just does not emit any requires which is what we have now.
:main my-app.feature1
:main [my-app.feature1]
:main [my-app.feature1 my-app.feature2]
Hide
Andrew Rosa added a comment -

@Thomas no defined :main will still will include base.js and deps.js, but not requiring anything. Right? At least is that what I would expect.

Yeah, tooling is very hard.

Show
Andrew Rosa added a comment - @Thomas no defined :main will still will include base.js and deps.js, but not requiring anything. Right? At least is that what I would expect. Yeah, tooling is very hard.
Hide
Thomas Heller added a comment -

No :main might as well behave like it does now, this way the feature is completely optional and will not break any existing code. Thats usually worth something, don't know how much because as I said, :main is mandatory in shadow-build.

ClojureScript itself should probably be more conservative than that. I imagine there are alot of projects that still use /out/goog/base.js + other in their HTML, that won't change over night.

Show
Thomas Heller added a comment - No :main might as well behave like it does now, this way the feature is completely optional and will not break any existing code. Thats usually worth something, don't know how much because as I said, :main is mandatory in shadow-build. ClojureScript itself should probably be more conservative than that. I imagine there are alot of projects that still use /out/goog/base.js + other in their HTML, that won't change over night.
Hide
David Nolen added a comment -

The approaches taken thus far are unnecessarily complex. All that needs to happen is for the generated :none script to document.write the needed scripts and goog.require the :main namespace. I will take a patch that does this.

Show
David Nolen added a comment - The approaches taken thus far are unnecessarily complex. All that needs to happen is for the generated :none script to document.write the needed scripts and goog.require the :main namespace. I will take a patch that does this.
Hide
Andrew Rosa added a comment -

Thanks @dnolen for the feedback. I will work on this patch.

Show
Andrew Rosa added a comment - Thanks @dnolen for the feedback. I will work on this patch.
Hide
David Nolen added a comment -

Andrew, I went ahead and tackled this one.

Show
David Nolen added a comment - Andrew, I went ahead and tackled this one.

People

Vote (0)
Watch (5)

Dates

  • Created:
    Updated:
    Resolved: