Closed
Bug 847013
Opened 11 years ago
Closed 11 years ago
Compiler should provide meta information on variables and globals used to compute the entity
Categories
(L20n :: JS Library, defect, P2)
Tracking
(Not tracked)
RESOLVED
FIXED
1.0 beta
People
(Reporter: zbraniecki, Assigned: stas)
References
Details
Attachments
(1 file)
2.28 KB,
patch
|
Details | Diff | Splinter Review |
This will enable us to trigger node retranslation on changes. See bug 847012 for the work on context/globals/html bindings side.
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → stas
Priority: -- → P2
Target Milestone: --- → 1.0 beta
Assignee | ||
Comment 1•11 years ago
|
||
G, something like this? I tested this quite extensively with some async scenarios using setTimeout and process.nextTick in node, and it didn't break. Our gets are around 2% slower compared to the code which didn't collect globals references.
Attachment #725133 -
Flags: feedback?(gandalf)
Reporter | ||
Comment 2•11 years ago
|
||
Comment on attachment 725133 [details] [diff] [review] Patch proposal Review of attachment 725133 [details] [diff] [review]: ----------------------------------------------------------------- ::: lib/compiler.js @@ +293,5 @@ > + _references.globals = {}; > + return { > + value: this.toString(ctxdata), > + globals: _references.globals, > + } document pls that this.toString rebuilds globals @@ +455,4 @@ > throw new RuntimeError('Reference to an unknown global: ' + name, > entry); > } > + _references.globals[name] = true; if that's all we store, would it make sense to keep globals as a list instead?
Assignee | ||
Comment 3•11 years ago
|
||
(In reply to Zbigniew Braniecki [:gandalf] from comment #2) > Comment on attachment 725133 [details] [diff] [review] > Patch proposal > > Review of attachment 725133 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: lib/compiler.js > @@ +293,5 @@ > > + _references.globals = {}; > > + return { > > + value: this.toString(ctxdata), > > + globals: _references.globals, > > + } > > document pls that this.toString rebuilds globals Right. > > @@ +455,4 @@ > > throw new RuntimeError('Reference to an unknown global: ' + name, > > entry); > > } > > + _references.globals[name] = true; > > if that's all we store, would it make sense to keep globals as a list > instead? I thought about this but then we'd need to de-duplicate the list (or have an implementation of a Python-like set?) so that we don't set too many listeners. Did I get that right?
Reporter | ||
Comment 4•11 years ago
|
||
Umm, can we use ES6 Set? :) http://www.nczonline.net/blog/2012/09/25/ecmascript-6-collections-part-1-sets/ Fx and Chrome support it
Assignee | ||
Comment 5•11 years ago
|
||
I'd love to use more ES6, or even some extensions to ES5, but for compatibility with node, we should stick to pure ES5.
Reporter | ||
Comment 6•11 years ago
|
||
good point, let's stick to that for now then
Assignee | ||
Updated•11 years ago
|
Flags: blocking-parkcity+
Assignee | ||
Comment 7•11 years ago
|
||
With bug 850855 fixed, I can now land this, as the function signatures are now compatible.
Assignee | ||
Comment 8•11 years ago
|
||
https://github.com/l20n/l20n.js/commit/c23640e2e10716ae1331da0be463b427bf3aa510
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•11 years ago
|
Attachment #725133 -
Flags: feedback?(gandalf)
You need to log in
before you can comment on or make changes to this bug.
Description
•