In LH ticket #433, Tobie says there are (new) style rules being used
in the rewrite effort that new contributions should adhere to. I've
said that it's hard to conform to a style guide that doesn't exist,
and suggested that a (brief) document saying what the goals and
rationales are, giving the specifics, references, etc., would make
sense -- for new contributors and current ones alike. Tobie agreed
and asked for volunteers.
Thought I'd widen the audience: Is there anyone who knows the new
rules and can write them up? Then we can link to them from the
Contribute page so people know what's going on.
--
T.J. Crowder
tj / crowder software / com
Before we go ahead and document the styles, I'd like to have a bit
more input (with relevant example code, please!) of how we should
handle feature testing.
Here's the originally proposed implementation for String#escapeHTML:
> In LH ticket #433, Tobie says there are (new) style rules being used
> in the rewrite effort that new contributions should adhere to. I've
> said that it's hard to conform to a style guide that doesn't exist,
> and suggested that a (brief) document saying what the goals and
> rationales are, giving the specifics, references, etc., would make
> sense -- for new contributors and current ones alike. Tobie agreed
> and asked for volunteers.
> Thought I'd widen the audience: Is there anyone who knows the new
> rules and can write them up? Then we can link to them from the
> Contribute page so people know what's going on.
> --
> T.J. Crowder
> tj / crowder software / com
That code looks a bit off -- like maybe at some point it used to have
entities that something, somewhere rendered as characters? Even the
raw version has this for the escapeHTMLWithReplace version:
> Before we go ahead and document the styles, I'd like to have a bit
> more input (with relevant example code, please!) of how we should
> handle feature testing.
> Here's the originally proposed implementation for String#escapeHTML:
> On Nov 12, 4:38 pm, "T.J. Crowder" <t...@crowdersoftware.com> wrote:
> > In LH ticket #433, Tobie says there are (new) style rules being used
> > in the rewrite effort that new contributions should adhere to. I've
> > said that it's hard to conform to a style guide that doesn't exist,
> > and suggested that a (brief) document saying what the goals and
> > rationales are, giving the specifics, references, etc., would make
> > sense -- for new contributors and current ones alike. Tobie agreed
> > and asked for volunteers.
> > Thought I'd widen the audience: Is there anyone who knows the new
> > rules and can write them up? Then we can link to them from the
> > Contribute page so people know what's going on.
> > --
> > T.J. Crowder
> > tj / crowder software / com
> That code looks a bit off -- like maybe at some point it used to have
> entities that something, somewhere rendered as characters? Even the
> raw version has this for the escapeHTMLWithReplace version:
> Just checking I'm not missing something obvious.
> -- T.J.
> On Nov 12, 4:05 pm, Tobie Langel <tobie.lan...@gmail.com> wrote:
> > Thanks for the write-up T.J.
> > Before we go ahead and document the styles, I'd like to have a bit
> > more input (with relevant example code, please!) of how we should
> > handle feature testing.
> > Here's the originally proposed implementation for String#escapeHTML:
> > On Nov 12, 4:38 pm, "T.J. Crowder" <t...@crowdersoftware.com> wrote:
> > > In LH ticket #433, Tobie says there are (new) style rules being used
> > > in the rewrite effort that new contributions should adhere to. I've
> > > said that it's hard to conform to a style guide that doesn't exist,
> > > and suggested that a (brief) document saying what the goals and
> > > rationales are, giving the specifics, references, etc., would make
> > > sense -- for new contributors and current ones alike. Tobie agreed
> > > and asked for volunteers.
> > > Thought I'd widen the audience: Is there anyone who knows the new
> > > rules and can write them up? Then we can link to them from the
> > > Contribute page so people know what's going on.
> > > --
> > > T.J. Crowder
> > > tj / crowder software / com
(1) I'm altogether in favor of this new approach, but it will result
in less "implicit" organization of code. A reader of the code will
likely find it harder to scan for the specific method they're looking
for. We'll probably have to address this by marking code "sections"
with comments.
(2) Also, it now becomes less clear where a PDoc block would go.
Technically, it would go at the bottom, where the external API is
created, but that'd separate it from the "logic" of the function and
would therefore make it more likely for the code and the docs to get
out of sync.
(Perhaps PDoc blocks can be used to demarcate, thereby solving both of
these problems.)
(3) Simple capability/quirk checks, as in the String#escapeHTML
example, can go directly into the conditional. More complex checks,
like the kind kangax has been working on, will need to be wrapped in
anonymous functions and distilled into booleans. I'd prefer these
booleans to have the naming conventions of constants (e.g.,
TREATS_ATTRIBUTES_AS_EXPANDO_PROPERTIES) so that they can be
recognized easily in the code.
Cheers,
Andrew
On Nov 13, 6:28 am, "T.J. Crowder" <t...@crowdersoftware.com> wrote:
> (1) I'm altogether in favor of this new approach, but it will result
> in less "implicit" organization of code. A reader of the code will
> likely find it harder to scan for the specific method they're looking
> for. We'll probably have to address this by marking code "sections"
> with comments.
I would be in favor of having every non-trivial method declaration in
a separate "wrapper" function. This would make it easier to
distinguish methods and keep all the implementation details in one
scope (avoiding accidental name clashes, etc.). In other words, keep
only one method declaration (together with all the supplementary tests/
methods) in one anonymous function. If we do that, it also becomes
possible to get rid of `Object.extend` and a returning object:
NativeObject.prototype.method = (function(){
var isBugPresent = (function(){
// feature test something...
})();
function original(){};
function workaround(){};
if (isBugPresent) {
return workaround;
}
return original;
})();
On the other hand, there are so many possibilities here that I think
we should do what makes more sense. E.g. if two methods need to use
one helper function or one test, they should probably be combined
under one "wrapper":
(function(){
var isBuggy = (function(){
// ...
})();
function originalEscape(){};
function originalUnescape(){};
function workaroundEscape(){};
function workaroundUnescape(){};
My only concern about this pattern is memory usage. There are many
functions created during branching process, yet only one is returned/
assigned (and is then used throughout the rest of the application
life). Those alternative functions are stored in a closure (e.g. in a
closure of "final" function) and so consume memory; every method has a
burden of its "branching environment" (memory-wise). This wouldn't
happen if we were to use function expressions (rather than function
declarations), but then our methods wouldn't have identifiers:
var someMethod = (function(){
if (doTest()) {
return function(){
// original function
}
}
else return function(){
// alternative version
}
})();
I've never found this to be a problem (i.e. lack of function "names")
but Tobie says that having those is useful for debugging. I'm still
not sure if such clarity/convenience worth memory loss. Perhaps this
"loss" is a non-issue and I'm just being overly paranoid : ) I
definitely like how clean and maintainable such pattern looks, though.
[snip PDoc section]
> (3) Simple capability/quirk checks, as in the String#escapeHTML
> example, can go directly into the conditional. More complex checks,
> like the kind kangax has been working on, will need to be wrapped in
> anonymous functions and distilled into booleans. I'd prefer these
> booleans to have the naming conventions of constants (e.g.,
> TREATS_ATTRIBUTES_AS_EXPANDO_PROPERTIES) so that they can be
> recognized easily in the code.
Agreed about capital names. I'm not sure where we should keep feature
tests, though. On one hand, it makes sense to keep them all under
`Prototype.BrowserFeatures`. On the other - it might be a bit
disturbing not to see the actual test implementation in the
appropriate method declaration (where all the branching occurs).
If a feature test ends up being used in various sections (dom, event,
string..) deciding on proper placement could become a problem. I think
it's easier to keep feature tests in one place.
Prototype.BrowserFeatures doesn't seem like the right place for those
tests since both features and bugs are tested for. The content of
Prototype.BrowserFeatures is currently camelcase, that wouldn't play
well with the uppercase names.
How about not having to run every single one of those tests upfront?
If tests are defined in let's say Feature.tests they can be run only
when required instead of upfront. That would prevent running complex
tests you don't end up using.
Here's a possible implementation using some of kangax' tests:
> > (1) I'm altogether in favor of this new approach, but it will result
> > in less "implicit" organization of code. A reader of the code will
> > likely find it harder to scan for the specific method they're looking
> > for. We'll probably have to address this by marking code "sections"
> > with comments.
> I would be in favor of having every non-trivial method declaration in
> a separate "wrapper" function. This would make it easier to
> distinguish methods and keep all the implementation details in one
> scope (avoiding accidental name clashes, etc.). In other words, keep
> only one method declaration (together with all the supplementary tests/
> methods) in one anonymous function. If we do that, it also becomes
> possible to get rid of `Object.extend` and a returning object:
> NativeObject.prototype.method = (function(){
> var isBugPresent = (function(){
> // feature test something...
> })();
> function original(){};
> function workaround(){};
> On the other hand, there are so many possibilities here that I think
> we should do what makes more sense. E.g. if two methods need to use
> one helper function or one test, they should probably be combined
> under one "wrapper":
> (function(){
> var isBuggy = (function(){
> // ...
> })();
> function originalEscape(){};
> function originalUnescape(){};
> function workaroundEscape(){};
> function workaroundUnescape(){};
> My only concern about this pattern is memory usage. There are many
> functions created during branching process, yet only one is returned/
> assigned (and is then used throughout the rest of the application
> life). Those alternative functions are stored in a closure (e.g. in a
> closure of "final" function) and so consume memory; every method has a
> burden of its "branching environment" (memory-wise). This wouldn't
> happen if we were to use function expressions (rather than function
> declarations), but then our methods wouldn't have identifiers:
> var someMethod = (function(){
> if (doTest()) {
> return function(){
> // original function
> }
> }
> else return function(){
> // alternative version
> }
> })();
> I've never found this to be a problem (i.e. lack of function "names")
> but Tobie says that having those is useful for debugging. I'm still
> not sure if such clarity/convenience worth memory loss. Perhaps this
> "loss" is a non-issue and I'm just being overly paranoid : ) I
> definitely like how clean and maintainable such pattern looks, though.
> [snip PDoc section]
> > (3) Simple capability/quirk checks, as in the String#escapeHTML
> > example, can go directly into the conditional. More complex checks,
> > like the kind kangax has been working on, will need to be wrapped in
> > anonymous functions and distilled into booleans. I'd prefer these
> > booleans to have the naming conventions of constants (e.g.,
> > TREATS_ATTRIBUTES_AS_EXPANDO_PROPERTIES) so that they can be
> > recognized easily in the code.
> Agreed about capital names. I'm not sure where we should keep feature
> tests, though. On one hand, it makes sense to keep them all under
> `Prototype.BrowserFeatures`. On the other - it might be a bit
> disturbing not to see the actual test implementation in the
> appropriate method declaration (where all the branching occurs).
> How about not having to run every single one of those tests upfront?
> If tests are defined in let's say Feature.tests they can be run only
> when required instead of upfront. That would prevent running complex
> tests you don't end up using.
Let's not prematurely optimize.
> I'm not sure where we should keep feature
> tests, though. On one hand, it makes sense to keep them all under
> `Prototype.BrowserFeatures`.
I actually don't think these should be made public.
Kangax, I'll get back to you regarding you closure based method
creation system later on today.
On Fri, 14 Nov 2008 01:59:07 +0300, kangax <kan...@gmail.com> wrote: > My only concern about this pattern is memory usage. There are many > functions created during branching process, yet only one is returned/ > assigned (and is then used throughout the rest of the application > life). Those alternative functions are stored in a closure (e.g. in a > closure of "final" function) and so consume memory; every method has a > burden of its "branching environment" (memory-wise). This wouldn't > happen if we were to use function expressions (rather than function > declarations), but then our methods wouldn't have identifiers: > var someMethod = (function(){ > if (doTest()) { > return function(){ > // original function > } > } > else return function(){ > // alternative version > } > })(); > I've never found this to be a problem (i.e. lack of function "names") > but Tobie says that having those is useful for debugging. I'm still > not sure if such clarity/convenience worth memory loss. Perhaps this > "loss" is a non-issue and I'm just being overly paranoid : ) I > definitely like how clean and maintainable such pattern looks, though.
Returning functions this way doesn't imply they do not have names. I've successfully used pattern "return someNamed function(){ ... }" for debugging.
> Returning functions this way doesn't imply they do not have names. I've
> successfully used pattern "return someNamed function(){ ... }" for
> debugging.
You mean "return function someNamed() { ... }", right? That's fine by
the specs, because the FunctionExpression is allowed to have an
optional name. (ECMA spec[1], section 13.)
> On Fri, 14 Nov 2008 01:59:07 +0300, kangax <kan...@gmail.com> wrote:
> > My only concern about this pattern is memory usage. There are many
> > functions created during branching process, yet only one is returned/
> > assigned (and is then used throughout the rest of the application
> > life). Those alternative functions are stored in a closure (e.g. in a
> > closure of "final" function) and so consume memory; every method has a
> > burden of its "branching environment" (memory-wise). This wouldn't
> > happen if we were to use function expressions (rather than function
> > declarations), but then our methods wouldn't have identifiers:
> > var someMethod = (function(){
> > if (doTest()) {
> > return function(){
> > // original function
> > }
> > }
> > else return function(){
> > // alternative version
> > }
> > })();
> > I've never found this to be a problem (i.e. lack of function "names")
> > but Tobie says that having those is useful for debugging. I'm still
> > not sure if such clarity/convenience worth memory loss. Perhaps this
> > "loss" is a non-issue and I'm just being overly paranoid : ) I
> > definitely like how clean and maintainable such pattern looks, though.
> Returning functions this way doesn't imply they do not have names. I've
> successfully used pattern "return someNamed function(){ ... }" for
> debugging.
> You mean "return function someNamed() { ... }", right? That's fine by the specs
Love it.
> Let's not prematurely optimize.
My proposal isn't just an optimization but a different approach for
the more complex tests used throughout the library. You asked for
feedback on how to handle feature testing, I think we could do better
then simply wrapping everything in closures. Here's what is comes down
to:
// complex test upfront in 'foo' closure.
var foo = (function(){
var REUSED_COMPLEX_FEATURETEST = (function(){
// ...
})();
if (test == simple) {
return function simpleWorkaround() {
// ..
};
}
if (REUSED_COMPLEX_FEATURETEST) {
return function complexWorkaround() {
// ..
};
}
//..
})();
Compared to Feature proposal:
// Reused complex test outside 'foo' closure.
var foo = (function() {
if (test == simple) {
return function simpleWorkaround() {
// ..
};
}
if (Feature.test('REUSED_COMPLEX_FEATURETEST')) {
return function complexWorkaround() {
// ..
};
}
// ..
})();
The optimizations are just an added bonus.
> I actually don't think these should be made public.
What's the harm there? Like with Prototype.BrowserFeatures it would
only make things easier internally.
It seems like you favour placing everything inside the different
closures as in my first example. What about complex tests used in more
then one place (dom, selector, array, etc.)?
A namespace like Feature could solve that issue. Perhaps it's not even
an issue right now though but having complex feature tests in one
place with documentation seems more maintainable in the long run.
sorry, Kangax for the delay in getting back to you. My computer turned
off before I could click send last night.
Of course, we can return named function.
What I'd want to avoid, however, (for measurable[1] performance hit
_every time the function is called_) is multiple closures.
So we should be avoiding the following construct:
(function() {
var index = 1;
return (function() {
if (SOME_FEATURE_TEST) {
return function namedFunc() { return index }
}
return function namedFunc2() { return index + 1 }
})();
On Nov 15, 4:56 am, "T.J. Crowder" <t...@crowdersoftware.com> wrote:
> > Returning functions this way doesn't imply they do not have names. I've
> > successfully used pattern "return someNamed function(){ ... }" for
> > debugging.
> You mean "return function someNamed() { ... }", right? That's fine by
> the specs, because the FunctionExpression is allowed to have an
> optional name. (ECMA spec[1], section 13.)
I stopped using named expressions after finding out about IE bug (what
else is new, right?). IE actually does something very stupid. It
erroneously parses named FunctionExpression as *both*
FunctionExpression and FunctionDeclaration. As surprising as it might
sound, IE creates 2 Function objects and so we, ironically, end up
with even more memory clog : )
This issue is more known for its "scoping" problem - when IE exposes
named expression identifier to the enclosing scope (since it's a
FunctionDeclaration), but I think that consequences of creating more
functions than needed is equally bad (and is exactly why I'm trying to
avoid it lately).
You can find more examples/explanations of this bug in c.l.j archives
but a simple test describing the problem would be:
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN"
"http://www.w3.org/TR/html4/strict.dtd">
<html lang="en">
<head>
<meta http-equiv="Content-Type" content="text/html;
charset=utf-8">
<title></title>
</head>
<body>
<script type="text/javascript">
(function(){
// this should produce undefined,
// as FunctionExpression's identifier (`foo`) should
// only be available to function body
// IE erroneously parses it as a FunctionDeclaration
document.write(foo + '<br>');
// create named FunctionExpression
var bar = (function foo(){
return bar === foo;
});
// foo simply compares Function object
// refered to by `foo` and Function object
// refered to by `bar`
// in IE it returns `false` -
// those are 2 different objects
document.write(foo());
> sorry, Kangax for the delay in getting back to you. My computer turned
> off before I could click send last night.
> Of course, we can return named function.
> What I'd want to avoid, however, (for measurable[1] performance hit
> _every time the function is called_) is multiple closures.
> So we should be avoiding the following construct:
> (function() {
> var index = 1;
> return (function() {
> if (SOME_FEATURE_TEST) {
> return function namedFunc() { return index }
> }
> return function namedFunc2() { return index + 1 }
> })();
> })()
Interesting.
It's also surprising that one more scope in a scope chain could
actually affect performance noticeably. I'll definitely take a closer
look at this (and maybe do some tests) when I get a chance.
On Nov 15, 10:00 am, Tobie Langel <tobie.lan...@gmail.com> wrote:
> > > I actually don't think these should be made public.
> > What's the harm there? Like with Prototype.BrowserFeatures it would
> > only make things easier internally.
> Whenever you make something public, you have to continue supporting
> it. This makes refactoring more complex.
I agree that we should be cautious when introducing anything "public".
I just wouldn't consider `BrowserFeatures` part of a public API. Those
are just a set of environment "descriptors". There's no defined
behavior and so nothing to "continue to support" in the future : ) If
anything, providing those booleans gives users a great set of low-
level tools to adopt and tailor their applications to a particular
environment.
I'm also altogether with this incoming pattern.
PDoc blocks could indeed demarcate those sections, and be used to
provide (browser|feature)-specific technical documentation.
My proposal is the following (http://gist.github.com/25421), where by
method appear :
- classical implementation, with "real" name.
- alternative implementations (with meaningful suffixes:
"UsingThis", "FixingThat")
- complex tests in named functions, suffixed by "ShouldUseThis",
"ShouldFixThat" as convention ?
- branching, i.e. replacing function referenced by final name,
depending on the return value of complex tests and inline trivial
tests.
It also shows an imaginary "implementation of:" PDoc tag.
About "complex shared tests", what are actual real cases ?
Are such tests shared among parts (say files in rewrite branch) or
pretty much isolated ?
A solution might be to declare all common tests first, not really
consistent with my previous proposition though...
Another interresting point is usage of "early or late binding" when
calling methods from other methods within the same closure (i.e.
http://gist.github.com/25430)
Early binding makes Prototype methods sort of frozen, but it looses
some dynamism.
Another better example is Enumerable, when all methods are written in
term of `each` or an higher-level iterator.
Should we write `this.each(...)` in `collect` code, or `each.call
(this, ...)` ?
`_each` being the only method that really has to be late-bound.
Best,
Samuel Lebeau
On 13 nov, 20:25, Andrew Dupont <goo...@andrewdupont.net> wrote:
> (1) I'm altogether in favor of this new approach, but it will result
> in less "implicit" organization of code. A reader of the code will
> likely find it harder to scan for the specific method they're looking
> for. We'll probably have to address this by marking code "sections"
> with comments.
> (2) Also, it now becomes less clear where a PDoc block would go.
> Technically, it would go at the bottom, where the external API is
> created, but that'd separate it from the "logic" of the function and
> would therefore make it more likely for the code and the docs to get
> out of sync.
> (Perhaps PDoc blocks can be used to demarcate, thereby solving both of
> these problems.)
> (3) Simple capability/quirk checks, as in the String#escapeHTML
> example, can go directly into the conditional. More complex checks,
> like the kind kangax has been working on, will need to be wrapped in
> anonymous functions and distilled into booleans. I'd prefer these
> booleans to have the naming conventions of constants (e.g.,
> TREATS_ATTRIBUTES_AS_EXPANDO_PROPERTIES) so that they can be
> recognized easily in the code.
> Cheers,
> Andrew
> On Nov 13, 6:28 am, "T.J. Crowder" <t...@crowdersoftware.com> wrote:
> > Hi Tobie,
> > > I'm actually interested in the pattern, not in the actual code.
> > Yeah, just wanted to make sure we were looking at the right code
> > first.
> > > I investigated a bit more re the typos you mentioned. It happens to be
> > > an issue with gist which... escaped some of the strings.
> > Lovely.
> > So stating the obvious: The basics are:
> > 1. Use named functions for each variant.
> > 2. Define the functions up front.
> > 3. Test proactive, in an appropriate order, and use the first one that
> > works correctly.
> Another interresting point is usage of "early or late binding" when
> calling methods from other methods within the same closure (i.e.http://gist.github.com/25430)
> Early binding makes Prototype methods sort of frozen, but it looses
> some dynamism.
In your first example:
function delay(timeout) {
var args = slice.call(arguments, 1);
__method = curry.apply(this, args);
return window.setTimeout(__method, timeout * 1000);
}
Where is `curry` "taken from"? Is it an "original" `curry` stored
somewhere in a closure?
> Another better example is Enumerable, when all methods are written in
> term of `each` or an higher-level iterator.
> Should we write `this.each(...)` in `collect` code, or `each.call
> (this, ...)` ?
If we use original `each` in `collect` and then I "wrap" `each` to log
its arguments, it would be surprising not to see `collect` logging my
arguments (as it is calling original `each`, rather than the one I
modified : ))
> `_each` being the only method that really has to be late-bound.
<samuel.leb...@gmail.com> wrote:
> Here is my take,
> I'm also altogether with this incoming pattern.
> PDoc blocks could indeed demarcate those sections, and be used to
> provide (browser|feature)-specific technical documentation.
> My proposal is the following (http://gist.github.com/25421)
I like it.
One nitpick: only one "implementation" of a particular method needs to
be PDoc'd, since we're documenting the public API and don't need to
inform the user about the internal differences between
implementations. We'd still have comments explaining what those
differences are, but they wouldn't be PDoc comments.
