ClojureScript

Include :js-dependency-index in result of `(env/default-compiler-env)`

Details

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

Description

It looks like the change from CLJS-615 requires the addition of something like this prior to using any of the dependency machinery:

(swap! env/*compiler* assoc :js-dependency-index (cljsc/js-dependency-index opts))

That the dependency index is now in the compiler environment isn't a problem (on the contrary), but cljs.env/default-compiler-env is there for exactly this sort of thing: the environment that fn returns should have :js-dependency-index added already. This would necessitate pulling all of the JS dependency resolution machinery up into cljs.env (or perhaps another namespace, cljs.js-deps?), but would eliminate the user-facing complication of getting a useful compiler environment with a side benefit of getting all the JavaScript dependency stuffs out of cljs.closure and into a home of their own.

I'm mucking with this in conjunction w/ CLJS-656; shouldn't be difficult to do afterwards.

Interested?

Activity

Chas Emerick made changes -
Field Original Value New Value
Description It looks like this change requires the addition of something like this prior to using any of the dependency machinery:

{code}
(swap! env/*compiler* assoc :js-dependency-index (cljsc/js-dependency-index opts))
{code}

That the dependency index is now in the compiler environment isn't a problem (on the contrary), but {{cljs.env/default-compiler-env}} is there for exactly this sort of thing: the environment that fn returns should have {{:js-dependency-index}} added already. This would necessitate pulling all of the JS dependency resolution machinery up into {{cljs.env}} (or perhaps another namespace, {{cljs.js-deps}}?), but would eliminate the user-facing complication of getting a useful compiler environment with a side benefit of getting all the JavaScript dependency stuffs out of {{cljs.closure}} and into a home of their own.

I'm mucking with this in conjunction w/ CLJS-656; shouldn't be difficult to do afterwards.

Interested?
It looks like this change from CLJS-615 requires the addition of something like this prior to using any of the dependency machinery:

{code}
(swap! env/*compiler* assoc :js-dependency-index (cljsc/js-dependency-index opts))
{code}

That the dependency index is now in the compiler environment isn't a problem (on the contrary), but {{cljs.env/default-compiler-env}} is there for exactly this sort of thing: the environment that fn returns should have {{:js-dependency-index}} added already. This would necessitate pulling all of the JS dependency resolution machinery up into {{cljs.env}} (or perhaps another namespace, {{cljs.js-deps}}?), but would eliminate the user-facing complication of getting a useful compiler environment with a side benefit of getting all the JavaScript dependency stuffs out of {{cljs.closure}} and into a home of their own.

I'm mucking with this in conjunction w/ CLJS-656; shouldn't be difficult to do afterwards.

Interested?
Chas Emerick made changes -
Description It looks like this change from CLJS-615 requires the addition of something like this prior to using any of the dependency machinery:

{code}
(swap! env/*compiler* assoc :js-dependency-index (cljsc/js-dependency-index opts))
{code}

That the dependency index is now in the compiler environment isn't a problem (on the contrary), but {{cljs.env/default-compiler-env}} is there for exactly this sort of thing: the environment that fn returns should have {{:js-dependency-index}} added already. This would necessitate pulling all of the JS dependency resolution machinery up into {{cljs.env}} (or perhaps another namespace, {{cljs.js-deps}}?), but would eliminate the user-facing complication of getting a useful compiler environment with a side benefit of getting all the JavaScript dependency stuffs out of {{cljs.closure}} and into a home of their own.

I'm mucking with this in conjunction w/ CLJS-656; shouldn't be difficult to do afterwards.

Interested?
It looks like the change from CLJS-615 requires the addition of something like this prior to using any of the dependency machinery:

{code}
(swap! env/*compiler* assoc :js-dependency-index (cljsc/js-dependency-index opts))
{code}

That the dependency index is now in the compiler environment isn't a problem (on the contrary), but {{cljs.env/default-compiler-env}} is there for exactly this sort of thing: the environment that fn returns should have {{:js-dependency-index}} added already. This would necessitate pulling all of the JS dependency resolution machinery up into {{cljs.env}} (or perhaps another namespace, {{cljs.js-deps}}?), but would eliminate the user-facing complication of getting a useful compiler environment with a side benefit of getting all the JavaScript dependency stuffs out of {{cljs.closure}} and into a home of their own.

I'm mucking with this in conjunction w/ CLJS-656; shouldn't be difficult to do afterwards.

Interested?
Hide
David Nolen added a comment -

I'm ok with pulling this stuff into it's own namespace.

Show
David Nolen added a comment - I'm ok with pulling this stuff into it's own namespace.
David Nolen made changes -
Priority Major [ 3 ] Minor [ 4 ]
Hide
Chas Emerick added a comment -

Attached patch refactors goog-style JS dependency stuff into cljs.js-deps, and changes cljs.env/default-compiler-env to populate :js-dependency-index based on the provided set of options.

I didn't move the externs stuff, seemed like it should stay in cljs.closure.

Note that this doesn't change the fact that CLJS compiler API consumers will need to change: cljs.env/default-compiler-env now takes a map argument of options, which needs to be provided if such consumers need to have their e.g. :libs, :foreign-libs options picked up. The advantage is default-compiler-env again does the job it's intended to do. The refactoring / slimming of cljs.closure is a nice side effect, tho.

Show
Chas Emerick added a comment - Attached patch refactors goog-style JS dependency stuff into cljs.js-deps, and changes cljs.env/default-compiler-env to populate :js-dependency-index based on the provided set of options. I didn't move the externs stuff, seemed like it should stay in cljs.closure. Note that this doesn't change the fact that CLJS compiler API consumers will need to change: cljs.env/default-compiler-env now takes a map argument of options, which needs to be provided if such consumers need to have their e.g. :libs, :foreign-libs options picked up. The advantage is default-compiler-env again does the job it's intended to do. The refactoring / slimming of cljs.closure is a nice side effect, tho.
Chas Emerick made changes -
Patch Code [ 10001 ]
Attachment CLJS-770.diff [ 12842 ]
David Nolen made changes -
Resolution Completed [ 1 ]
Status Open [ 1 ] Resolved [ 5 ]
David Nolen made changes -
Status Resolved [ 5 ] Closed [ 6 ]

People

Vote (1)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: