Vaadin-grid: [Polymer 3] Style modules broken in AMD build

Created on 5 Dec 2018  路  4Comments  路  Source: vaadin/vaadin-grid

Description

vaadin-grid-styles.js is broken in AMD builds.

Expected outcome

No errors or warning, and table styles are visually the same than in non-AMD builds.

Actual outcome

I get this console error:
SyntaxError: redeclaration of const $_documentContainer (vaadin-grid-styles.js)
And this corresponding warning:
Could not find style data in module named vaadin-grid-styles (style-gather.js)

And all the table styles are visually messed up, of course.

Live Demo

You can just open any of your demos with the following steps to reproduce

Steps to reproduce

  • Use "@vaadin/vaadin-grid": "^5.2.1" from npm
  • Run polymer serve --compile never
  • Open Firefox (which as of today requires an AMD build because it doesn't support dynamic imports)

Browsers Affected

  • [ ] Chrome
  • [X] Firefox
  • [ ] Safari
  • [X] Edge
  • [X] IE 11
  • [ ] iOS Safari
  • [ ] Android Chrome

Most helpful comment

vaadin-grid-styles.js is currently written with this format (the old polymer-modulizer format):

const $_documentContainer = document.createElement('template');

$_documentContainer.innerHTML = `<dom-module id="vaadin-grid-styles">
  <template>
    <style>
      ...
    </style>
  </template>
</dom-module>`;

document.head.appendChild($_documentContainer.content);

Per the polymer documentation (which was admittedly updated recently), it should be written this way:

import '@polymer/polymer/lib/elements/dom-module.js'

const VaadinGridStyles = document.createElement('dom-module')
VaadinGridStyles.innerHTML = `
  <template>
    <style>
      ...
    </style>
  </template>
`

VaadinGridStyles.register('vaadin-grid-styles')

I have verified that this rewriting fixes the issue. I would already have opened the PR if this file was in your source code but it appears not to be. This repo's sources are still written in Polymer 2, so I'm assuming you guys are running the polymer-modulizer at build time, this complicates things since it means this bug in your project won't be fixed until the polymer team finds the time to fix their bug in their project.

Since the modulizer is not yet fixed:

  • Consider converting your source to Polymer 3 in the next major version (breaking change) and fix the issue directly
    OR
  • As a workaround, add another build step that runs after the modulizer and applies this rewrite until the modulizer is fixed

All 4 comments

vaadin-grid-styles.js is currently written with this format (the old polymer-modulizer format):

const $_documentContainer = document.createElement('template');

$_documentContainer.innerHTML = `<dom-module id="vaadin-grid-styles">
  <template>
    <style>
      ...
    </style>
  </template>
</dom-module>`;

document.head.appendChild($_documentContainer.content);

Per the polymer documentation (which was admittedly updated recently), it should be written this way:

import '@polymer/polymer/lib/elements/dom-module.js'

const VaadinGridStyles = document.createElement('dom-module')
VaadinGridStyles.innerHTML = `
  <template>
    <style>
      ...
    </style>
  </template>
`

VaadinGridStyles.register('vaadin-grid-styles')

I have verified that this rewriting fixes the issue. I would already have opened the PR if this file was in your source code but it appears not to be. This repo's sources are still written in Polymer 2, so I'm assuming you guys are running the polymer-modulizer at build time, this complicates things since it means this bug in your project won't be fixed until the polymer team finds the time to fix their bug in their project.

Since the modulizer is not yet fixed:

  • Consider converting your source to Polymer 3 in the next major version (breaking change) and fix the issue directly
    OR
  • As a workaround, add another build step that runs after the modulizer and applies this rewrite until the modulizer is fixed

Filed #803 in polymer/tools about this

Thanks for the issue. We will consider tweaking our tool for proper replacement. The solution suggested via Polymer docs is something worth considering, and we also need to somehow workaround vaadin/vaadin-themable-mixin#36, ideally via same change

Was this page helpful?
0 / 5 - 0 ratings