669486 - Both the Style Inspctor and GCLI use the same template engine, but different copies of it

Status: VERIFIED FIXED

(DevTools :: General, defect)

Comment 1

[Joe Walker [:jwalker] (needinfo me or ping on irc)]

Here's what I think we need to do:
- Tweak it so the implementation is static
  (i.e. a single function or set of functions)
- Move it out into a JSM
- Have the Style Inspector import it
- For GCLI:
  - Tweak dryice to allow several implementation of the same module
  - Create a new templater module that just defers to a JSM
  - Ensure that we can package a built in templater for html
    and the proxied one for firefox
- Ensure the in-tree version is commented to point out the other
  versions of the same code

Comment 2

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

In my naivety I thought that once the templater was in a JSM both tools could import and use it. I guess the GCLI is a special case :o/

Comment 3

[Joe Walker [:jwalker] (needinfo me or ping on irc)]

If only we had Harmony modules, this would be easy ....

Comment 4

[Rob Campbell [:rc] (:robcee)]

why can't you use the JSM?

Comment 5

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

(In reply to comment #4)
> why can't you use the JSM?

Because gcli is also used in other web apps (such as ace).

Comment 6

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

(In reply to comment #1)
> Here's what I think we need to do:
> - Tweak it so the implementation is static
>   (i.e. a single function or set of functions)
> - Move it out into a JSM
> - Have the Style Inspector import it
> - For GCLI:
>   - Tweak dryice to allow several implementation of the same module
>   - Create a new templater module that just defers to a JSM
>   - Ensure that we can package a built in templater for html
>     and the proxied one for firefox
> - Ensure the in-tree version is commented to point out the other
>   versions of the same code

Or just have the Firefox dryice build ignore the templater requirement, and have a check in the code like this:

var DomTemplate = require('path/to/dom-template');
if (!DomTemplate) {
  if (typeof Components === 'object'
      && typeof Components.utils === 'object'
      && typeof Components.utils.import === 'funciton') {
    Components.utils.import('resource:///path/to/dom-template');
  } else {
    throw new Error('blah blah blah');
  }
}

Not sure if that is cleaner though...

Comment 7

[Joe Walker [:jwalker] (needinfo me or ping on irc)]

Or half way:
- Have dryice ignore the templater requirement
- Add to prefix-gcli.jsm:

var x = {};
Components.utils.import('resource:///path/to/dom-template', x);
define('path/to/dom-template', [], function(exports) {
  exports.DomTemplate = x.DomTemplate;
});

That way all the client code can just import and be done.

Comment 8

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

That seems best to me, for sure.

Comment 9

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Joe: I have already moved DOM Templater into a jsm as part of bug 582596. I will submit the new patch in the next couple of days.

Comment 10

[Joe Walker [:jwalker] (needinfo me or ping on irc)]

Thanks Mike - please tell me when I can sync up.

Comment 11

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Okay ... I will move it to devtools/shared and let you know when Dolske approves the patch.