Polymer / pwa-starter-kit

Starter templates for building full-featured Progressive Web Apps from web components.
https://pwa-starter-kit.polymer-project.org
2.36k stars 431 forks source link

Deleting `nav` with class `toolbar-list` in app-header causes error #345

Closed renatavmaraj closed 5 years ago

renatavmaraj commented 5 years ago

Getting an error when deleting the first <nav> component in the main my-app file from the Typescript template.

Steps to reproduce:

Delete or comment out the following code and related css in main my-app file:

<nav class="toolbar-list">
  <a ?selected="${this._page === 'view1'}" href="/view1">View One</a>
  <a ?selected="${this._page === 'view2'}" href="/view2">View Two</a>
  <a ?selected="${this._page === 'view3'}" href="/view3">View Three</a>
</nav> 

Check console, and you will see:

Uncaught (in promise) TypeError: Cannot read property 'split' of null
    at _prepareTemplate (template.ts:109)
    at new Template (template.ts:203)
    at Object.result [as templateFactory] (shady-render.ts:94)
    at NodePart._commitTemplateResult (parts.ts:285)
    at NodePart.commit (parts.ts:230)
    at TemplateInstance.update (template-instance.ts:60)
    at NodePart._commitTemplateResult (parts.ts:299)
    at NodePart.commit (parts.ts:230)
    at render (render.ts:57)
    at Function.render (shady-render.ts:284)
keanulee commented 5 years ago

Couldn't reproduce - did you miss any brackets or quotes or something?

diff --git a/src/components/my-app.ts b/src/components/my-app.ts
index 6b3d7e1..ff1055f 100644
--- a/src/components/my-app.ts
+++ b/src/components/my-app.ts
@@ -104,23 +104,6 @@ export class MyApp extends connect(store)(LitElement) {
           padding-right: 44px;
         }

-        .toolbar-list {
-          display: none;
-        }
-
-        .toolbar-list > a {
-          display: inline-block;
-          color: var(--app-header-text-color);
-          text-decoration: none;
-          line-height: 30px;
-          padding: 4px 24px;
-        }
-
-        .toolbar-list > a[selected] {
-          color: var(--app-header-selected-color);
-          border-bottom: 4px solid var(--app-header-selected-color);
-        }
-
         .menu-btn {
           background: none;
           border: none;
@@ -179,10 +162,6 @@ export class MyApp extends connect(store)(LitElement) {
         /* Wide layout: when the viewport width is bigger than 460px, layout
         changes to a wide layout */
         @media (min-width: 460px) {
-          .toolbar-list {
-            display: block;
-          }
-
           .menu-btn {
             display: none;
           }
@@ -210,13 +189,6 @@ export class MyApp extends connect(store)(LitElement) {
           <button class="menu-btn" title="Menu" @click="${this._menuButtonClicked}">${menuIcon}</button>
           <div main-title>${this.appTitle}</div>
         </app-toolbar>
-
-        <!-- This gets hidden on a small screen-->
-        <nav class="toolbar-list">
-          <a ?selected="${this._page === 'view1'}" href="/view1">View One</a>
-          <a ?selected="${this._page === 'view2'}" href="/view2">View Two</a>
-          <a ?selected="${this._page === 'view3'}" href="/view3">View Three</a>
-        </nav>
       </app-header>

       <!-- Drawer content -->
renatavmaraj commented 5 years ago

No, didn't miss any brackets or quotes. I even just tried cloning it a third time, and doing the same thing. Same error. @keanulee Are you using the typescript-template?

keanulee commented 5 years ago

Yes, the above diff was applied on commit 2d85aa1.

I don't really see why removing just those things would cause that error - did you make any other changes? Does it work if you undo that change? Do you have a repo I can look at?

renatavmaraj commented 5 years ago

@keanulee Just tried this again, and still doesn't work. If I undo the change, it does work. I've made a public repo for you to check out: https://github.com/renatavmaraj/pwa-typescript-template

logicalphase commented 5 years ago

I cloned your repo, and I can confirm that if you comment out that section it's throwing the same split error.

template.ts:109 Uncaught (in promise) TypeError: Cannot read property 'split' of null
    at _prepareTemplate (template.ts:109)
    at new Template (template.ts:203)
    at Object.templateFactory (shady-render.ts:94)
    at NodePart._commitTemplateResult (parts.ts:285)
    at NodePart.commit (parts.ts:230)
    at render (render.ts:57)
    at Function.render (shady-render.ts:284)
    at HTMLElement.update (lit-element.ts:232)
    at HTMLElement.performUpdate (updating-element.ts:743)
    at HTMLElement._enqueueUpdate (updating-element.ts:687)

However, if I delete the nav section completely, it does render correctly. I've attached two screenshots to attest to the latter.

pwa-ts-template-test

pwa-ts-template-test-screen

keanulee commented 5 years ago

Ah, you can't have lit-html bindings inside comment nodes, so this isn't allowed:

html`<!-- ${someValue} -->`