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

::template(true) -- too much recursion #3

Open
dspinhirne opened this issue May 26, 2017 · 24 comments
Open

::template(true) -- too much recursion #3

dspinhirne opened this issue May 26, 2017 · 24 comments

Comments

@dspinhirne
Copy link

Looking at your tests as examples for using the code and ran into an issue with templates. From your tests is seems that providing the "true" arg to a template should cause it to render automatically, however in practice this leads to a recursion error being raised. Using code from your basic test case, registering it, and using it in an html file:

var component1 = xtag.create(class extends XTagElement {
'::template(true)'(){
	return `<h1>title auto 1</h1><p>content auto 1</p>`;
}
});
xtag.register('x-component1', component1);


--
<x-component1></x-component1>

Raises this issue in the js console.

@csuwildcat
Copy link
Member

Looking into this now.

@csuwildcat
Copy link
Member

@dspinhirne I can't seem to raise this error when I run the tests in any browser I am testing with. Are you saying it is behaving differently outside of the test spec?

@dspinhirne
Copy link
Author

Yes, this is outside the test spec. I am attempting to use templates as part of one of my own custom components and hit the issue.

In order to verify my own work I used code from your tests and hit the same issue. Basically, I created a js file and copied the code for one of your testing custom components into it. I then created an html file and attempted to use the custom element (x-component1) as shown above. This is where I hit the issue.

@ghost
Copy link

ghost commented Sep 18, 2017

I'm sorry this is late @csuwildcat but I ran into the same problem with using templates.

@csuwildcat and @dspinhirne, I've got it to work, here is the code below.

var _temp = xtag.create(class _temp extends XTagElement {
'hello::template(true)'(){
return <h1>title1</h1><p>content1</p>;
}
});

var Fire = xtag.create(class Fire extends XTagElement{
connectedCallback (){
console.log("hi");
this.render("hello");
}
} );

xtag.register("fire-base", Fire);

@ghost
Copy link

ghost commented Sep 18, 2017

@csuwildcat I guess my only question is, what does the true parameter do? In your specs.js file it says:

"auto-render templates marked with the option", func...

Is this supposed to work like v1's content: property?

@csuwildcat
Copy link
Member

csuwildcat commented Sep 18, 2017 via email

@ghost
Copy link

ghost commented Sep 18, 2017

Yeah no problem, @csuwildcat

Did you want me to test the content at https://github.com/x-tag/core/tree/v2 -- edit below

I opened the index.html file at,
https://github.com/x-tag/core-2/blob/master/tests/basic/index.html in this repository on my computer, is that what you mean by testing? I apologize if I'm misunderstanding.

If that's what you meant all those tests are clicking good, the recursion problem only occurs during the xtag.register() of a custom element (The tests inside, specs.js test the xtag.create() method but not template registrations.)

The ::template() method works if you just use the create method and give the template a name, and than manually use it with this.render(yourTemplate) in other elements.

When I tried to debug the issue using the constructor() method I got over 1000 console logs and than finally got the recursion error.

I don't know if any of this helps, but I'll keep plucking away at it.

@ghost
Copy link

ghost commented Sep 30, 2017

Tested out using the template again just now. It's still giving me a recursion error on Firefox 64 bit Nightly build most current edition, and is also giving me a similar error on MS Edge except it says out of out of stack space of course.

On Opera I get an error that points to the file containing the element with the error, Uncaught DOMException: Failed to construct 'CustomElement': The result must not have children.

-edit

Also, the errors I'm getting aren't from xtag source but from custom-elements.min.js

@ghost
Copy link

ghost commented Sep 30, 2017

Could this happen if you haven't patched the shadowDOM or polyfill? If it is please close the issue. Thank you.

@dspinhirne are you still having this problem. If you are just use the cdn to quickly resolve it or I think checking to make sure all the files in bower are installing correctly. @csuwildcat, does this sound correct?

Great job @csuwildcat

@ghost
Copy link

ghost commented Sep 30, 2017

I was just wondering @csuwildcat have you ever thought of switching to yarn?

@csuwildcat
Copy link
Member

csuwildcat commented Sep 30, 2017 via email

@ghost
Copy link

ghost commented Sep 30, 2017

Lol, no sorry it was just a personal thing. I was looking at Yarn and thought it was neat and clean. Was also wondering what the advantages would be exactly myself? Package locking I can see how can be useful but haven't really used it much myself

@csuwildcat
Copy link
Member

I think this is probably related to child insertion inside the class constructor, which I can change. The issue I have is that none of my tests fail in Chrome, Firefox, or Edge on Windows - can you provide a failing test setup?

@ghost
Copy link

ghost commented Sep 30, 2017

I was heading that way to, i wilk provide a build that fails on top of the xtag github site I havr forked when I get off work if that works for you.

@csuwildcat
Copy link
Member

csuwildcat commented Sep 30, 2017 via email

@ghost
Copy link

ghost commented Sep 30, 2017

Im not on my comp now or i would push the failing build now I will push the failing build when

@ghost
Copy link

ghost commented Sep 30, 2017

I get off, but I set to the script src "without cdn" and without the "./"

@ghost
Copy link

ghost commented Sep 30, 2017

In the index.html file in the site repo. It failed for me.

@ghost
Copy link

ghost commented Oct 1, 2017

Link to the repo I am using is here -> https://github.com/KipOmaha/x-tag-setup.

To get the build running you'll have to clone it (sounds wierd cause it don't run), but...
I decided to create a GH-page for it and it throws a different error.

@ghost
Copy link

ghost commented Oct 1, 2017

@ghost
Copy link

ghost commented Oct 1, 2017

The construcor's error I believe was preventing the recursive error from appearing, @csuwildcat.

@ghost
Copy link

ghost commented Oct 4, 2017

@csuwildcat, just wanted to update this thread.

I have a build with no recursion error and an altered render() method with no weird behaviors. (Testing in progress)

@csuwildcat
Copy link
Member

Can we move all these Issues to the regular Core repo? That is where the most up-to-date code is, and I want to try closing this repo out.

I don't see errors on this page in any of my browsers on Windows (Edge, Chrome, Firefox, etc.): https://kipomaha.github.io/x-tag-setup/index.html.

Were you able to generate a reduced test case? Preferably, we could isolate this down to a single test we can add to the test spec that would throw if this issue was present.

@ghost
Copy link

ghost commented Oct 6, 2017

@csuwildcat I can create a new jasmine.html with a reduced test, or I can add it direct to the spec.js test in the basic directory as a pull request to check for the issue, but at the moment I'm using a test library with a modified render method that gets rid of the error.

If you want this issue in core I would reccomend changing it to

recursion errors regarding this.innerHTML

This is where I`m finding recursion errors at least but it needs more testing.

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

No branches or pull requests

2 participants