erikringsmuth / app-router

Router for Web Components
https://erikringsmuth.github.io/app-router/
MIT License
610 stars 83 forks source link

Allow vulcanizing without having to specify the element attribute #22

Open thegecko opened 9 years ago

thegecko commented 9 years ago

Just a minor thing, it seems the element tag needs to be set for an imported component for vulcanizing to work even if the imported component has the same name as the file.

For example, I have a component in a file name home-page.html exported with a name of 'home-page'.

This code:

<app-route path="/" import="components/home-page.html"></app-route>

Results in polymer throwing an error when loaded after vulcanization:

Uncaught HierarchyRequestError: Failed to execute 'appendChild' on 'Node': Nodes of type 'HTML' may not be inserted inside nodes of type '#document'.

This code fixes it:

<app-route path="/" import="components/home-page.html" element="home-page"></app-route>

Cheers!

erikringsmuth commented 9 years ago

I can't reproduce this. I can run vulcanize and convert an element like this.

elements/test-el.html

<link rel="import" href="/bower_components/polymer/polymer.html">
<polymer-element name="test-el" noscript>
  <template>
    test
  </template>
</polymer-element>

to this

dist/test-el.html

<link rel="import" href="/bower_components/polymer/polymer.html">
<polymer-element name="test-el" assetpath="../elements/">
  <template>
    test
  </template>
<script>Polymer('test-el');</script></polymer-element>

and it loads just fine.

Do you know if the exception is being thrown from the app-router or Polymer?

thegecko commented 9 years ago

Hey Erik,

I've recreated this minimally locally and I need to apologise as I missed out one piece of the puzzle!

For this to break, you also need to import another component in the head of the page. Here's what you need for recreation:

build.html

<!DOCTYPE html>
<html lang="en">
<head>
    <title>app-router test</title>
    <script src="bower_components/platform/platform.js"></script>
    <link rel="import" href="components/dummy-page.html">
    <link rel="import" href="bower_components/app-router/app-router.html">
</head>

<body>

    <a href="#/test">off we go</a>

    <app-router>
        <app-route path="/test" import="components/test-page.html"></app-route>
    </app-router>

</body>
</html>

dummy-page.html

<link rel="import" href="../bower_components/polymer/polymer.html">

<polymer-element name="dummy-page">

    <template>
        <h1>Dummy</h1>
    </template>

    <script>
        Polymer({});
    </script>

</polymer-element>

test-page.html

<link rel="import" href="../bower_components/polymer/polymer.html">

<polymer-element name="test-page">

    <template>
        <h1>Test</h1>
    </template>

    <script>
        Polymer({});
    </script>

</polymer-element>

Cheers!

erikringsmuth commented 9 years ago

I see it now! This looks like a bug in Vulcanize or Polymer. I think the ../bower_components/ at the beginning of your paths is messing up Vulcanize. The vulcanized documents get a bunch of extra CSS when you go down a path level like that. Then later Polymer throws an error here when trying to load it.

polymer.concat.js

line 6284

// TODO(rafaelw): Remove when fix for
// https://codereview.chromium.org/164803002/
// makes it to Chrome release.

line 6290

var html = d.appendChild(d.createElement('html'));

I'm out of time to look at this now but I'll dig more eventually. A workaround would be to start your paths with /bower_components/....

JayBeavers commented 9 years ago

+1, just hit this as well. BTW @erikringsmuth I have to use the relative paths rather than absolute paths due to Cordova...

Thanks @thegecko for having a ready-fix :-)

davidcv5 commented 9 years ago

@erikringsmuth same here... great component, btw... thanks!

alexpaluzzi commented 9 years ago

+1! Just ran into it. I'll take a look and see if I can help.

Thanks

davidcv5 commented 9 years ago

Just thought I should mention that it pretty much depends on the structure of your app. Was having that issue and decided to start from scratch following Erik's samples and now it all works perfect.

@erikringsmuth I noticed that in the docs it says the following when vulcanizing..

screenshot 2015-02-24 14 05 48

It looks like only 'element' is required, but actually 'import' should be excluded when using a vulcanized element.

Again.. Thanks!!!

erikringsmuth commented 9 years ago

It's been a while since I debugged this issue but I think it had to do with vulcanize handling relative paths in imports.

<link rel="import" href="../my-component.html">

I think part of the issue is a bug in the vulcanize code. That said, there may also be a bug in the router.

piton4eg commented 9 years ago

@davidcv5 it works for me, thanks!