This project is archived and is in readonly mode.

#958 ✓invalid
Michael Ficarra

various better practices and standards-compliance suggestions

Reported by Michael Ficarra | August 11th, 2010 @ 06:02 AM | in 2.0 (closed)

I have made a gist containing a diff with some suggested changes to core. Take them or leave them, I will monitor this ticket to answer any questions about the suggestions.

http://gist.github.com/516654

Comments and changes to this ticket

  • Keeto

    Keeto August 18th, 2010 @ 07:56 PM

    Questions and comments:

    • [protect] Which browsers did you use to test this? Function.prototype.protect doesn't get turned into a generic until after it's processed by Type, which means you'll have to move it earlier in the force list to get your code to work. This won't be apparent unless you tested it in Firefox, which supports native generics for Array.
    • [override] What's the issue with override? It's not a reserved word, and I don't know any real issues regarding this particular identifier.
    • [Function] It's a global native constructor as defined by the ECMAScript specifications--it's always present. Always. If it's not there, then the environment is a non-conforming environment, which probably means it's not supported.
    • [callee] I agree with this, but I'm wondering why you're using Object.prototype.hasOwnProperty.call(item) instead of item.hasOwnProperty (3 property access operations versus 1).

    I'm in no position to comment on the other issues, so that is all. :>

  • Sebastian Markbåge

    Sebastian Markbåge August 18th, 2010 @ 08:28 PM

    I'm curious as to what real issues some of these solve. Do you have some edge case environment where you've hit an issue?

  • Michael Ficarra

    Michael Ficarra August 27th, 2010 @ 06:09 PM

    @Keeto:

    • [protect] I tried it on chrome/linux. I will take a deeper look into it soon.
    • [override] Actually, it's reserved for use in ES Harmony, the successor to ES5
    • [Function] It's always present, but can be changed or masked in a more local scope. The prototype.constructor property of a function literal, while not immutable, seems like a safer and more reliable handle to the Function constructor.
    • [callee] Again, I'd rather make as few assumptions as possible. Not all JavaScript values have the hasOwnProperty property available, and it can be overridden in those that do.

    @Sebastian: All of these changes came from using MooTools to test Google Caja. Some were required to get the test suite to pass, others remained good practice after a fix to a broken Caja feature was issued.

  • Sebastian Markbåge

    Sebastian Markbåge August 27th, 2010 @ 06:52 PM

    • [protect] If it isn't possible to extend Function.prototype properly or there is a conflicting method, then MooTools will fail anyway. That's the peril of extending prototypes.
    • [override] Can you supply a reference to a document that specifies this?
    • [Function] If Function is masked in a local scope it seems arbitrary to check that first. Since it would have to be overridden with a falsify value.
    • [callee] More precise, sure, but there's nothing to say an object can't have both those properties and break the assumption anyway. It's still fragile. More code, problem not solved.

    I'm not sure these "best" practices are generalizable beyond the bug in Caja.

  • Keeto

    Keeto August 28th, 2010 @ 12:01 PM

    All of my follow-up questions have been answered by Sebastian, so nothing more for me--except for one thing.

    And that's override. Harmony is not a standard yet--it's not even anything right now aside from plans. I'm not trying to lessen the value of the Harmony project, but we cannot base any decision on something currently so vague. ECMAScript 4 got into draft stage, but where is it now?

    Design decisions need to be made based on the current status of the technology and the merits of going through with such a decision--not simply because there's a potential that it will be part of the standards.

  • Sebastian Markbåge

    Sebastian Markbåge August 28th, 2010 @ 08:30 PM

    Additionally the T39 (Harmony) folks are explicitly avoiding introducing new keywords because it would break existing code. There are talks of introducing a versioning system which would help. I really doubt this is an issue. But if you have a case I'd love to hear about it.

  • Michael Ficarra

    Michael Ficarra August 29th, 2010 @ 01:32 AM

    • Title changed from “various "best practices" and standards-compliance suggestions” to “various better practices and standards-compliance suggestions”

    All I can say about the override keyword is that caja already disallows its use as an identifier and my mentor (Mike Stay) told me the reason for that was that it was reserved for ES Harmony. I can't attest to much more than that.

    As for arguments.callee access, it may not be more successful in detecting objects with "Arguments" as its internal [[Class]] property, but it will not throw errors when run in ES5 strict mode. This is a must.

  • Sebastian Markbåge

    Sebastian Markbåge August 31st, 2010 @ 04:56 PM

    • Milestone set to 1.3.1
    • State changed from “new” to “open”
    • Milestone order changed from “788” to “0”

    MooTools doesn't really fully support strict mode yet but it will.

  • Michael Ficarra

    Michael Ficarra September 11th, 2010 @ 06:41 AM

    I've just been notified that "override" is actually no longer a reserved word in the current ES Harmony spec, so that can be ignored.

  • Christoph Pojer

    Christoph Pojer October 14th, 2010 @ 09:37 AM

    • Assigned user set to “Sebastian Markbåge”

    Is there actually anything we are going to add from this? Sebastian, you wanna take a look?

  • Sebastian Markbåge

    Sebastian Markbåge October 15th, 2010 @ 09:51 AM

    Sure. This is dependent on MooTools going strict mode compatible.

  • Christoph Pojer

    Christoph Pojer November 9th, 2010 @ 06:54 PM

    • Milestone changed from 1.3.1 to 1.4.0
  • Christoph Pojer

    Christoph Pojer August 13th, 2011 @ 12:45 AM

    • Assigned user changed from “Sebastian Markbåge” to “Valerio”
    • Milestone changed from 1.4.0 to 2.0
    • Milestone order changed from “1” to “0”
  • ibolmo

    ibolmo January 19th, 2012 @ 09:59 AM

    • State changed from “open” to “invalid”

    Moved to Github Issues: https://github.com/mootools/mootools-core/issues/2234

    Please use Github Issues, since Lighthouse has been deprecated.

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Shared Ticket Bins

Pages