Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed error when working with prototypes #207

Closed
wants to merge 1 commit into from

Conversation

sokraflex
Copy link

Upon rendering any template, as long as it contains an element that has attributes, blade throws the error:
"Unable to read property text of undefined"
It turned out, that, when working with prototypes, blade appends an attribute called '' with the value undefined to the attribute list. Using the above fix, such attributes get removed from the attribute list before getting compiled, and the error does not occur.

This is only a workaround. I looked into the "lib/parser/index.js"-file, but couldn't find the line where the attributes are generated.

Upon rendering any template, as long as it contains an element that has attributes, blade throws the error:
"Unable to read property text of undefined"
It turned out, that, when working with prototypes, blade appends an attribute called '' with the value ```undefined``` to the attribute list. Using the above fix, such attributes get removed from the attribute list before getting compiled, and the error does not occur.

This is only a workaround. I looked into the "lib/parser/index.js"-file, but couldn't find the line where the attributes are generated.
@bminer
Copy link
Owner

bminer commented May 26, 2016

@SargTeX - Hmm... can you post an excerpt of your Blade template that produced an error? I'd like to reproduce this.

@sokraflex
Copy link
Author

It was reproducable on any template that has an html element with an
attribute list, even if the attribute list is empty, like in the following
one:

div(class="test")

You receive that result, if you add a method to the Array prototype:

Array.prototype.contains = function(element) {
    return this.indexOf(element) >= 0;
};

Another option to exclude such elements (rather than deleting them) would
be detecting them via hasOwnProperty:

if (!attributes.hasOwnProperty(name)) // prototype-functions return false
upon hasOwnProperty-call

Greetings

@bminer
Copy link
Owner

bminer commented May 27, 2016

Ah, indeed. This is probably because Blade is using for(var i in attributes) somewhere and picking up the added property. I'll look into it because it's probably a bug.

What happens if you override the Array prototype using Object.defineProperty with enumerable set to false?

@bminer
Copy link
Owner

bminer commented May 31, 2016

@SargTeX - This isn't really a bug as I originally thought, so I am closing this issue. When you override the Array prototype, you are asking for trouble, since almost all for(var i in arrayInstance) blocks break. This is because methods added to the Array prototype are now enumerable.

Consider this code:

Array.prototype.contains = function(element) {
    return this.indexOf(element) >= 0;
};
var arrayInstance = [1, 2, 3];
for(var i in arrayInstance) {
    console.log(i);
}

Here's what would be printed:

0
1
2
contains

Notice that contains was also printed to the console, which is probably not what you want. So, Instead of doing things like:

Array.prototype.contains = function(element) {
    return this.indexOf(element) >= 0;
};

... Do this instead:

Object.defineProperty(Array.prototype, "contains", {enumerable: false, value: function(element) {
    return this.indexOf(element) >= 0;
}});

A side note... to be fair, the "best" way IMHO to loop through an Array is to use the for(var i = 0; i < arrayInstance.length; i++) construct, but... some parts of the Blade codebase don't follow this "convention."

@bminer bminer closed this May 31, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants