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

modern-and-legacy-config not save correct index.html - missing preload and polyfills #435

Closed
mjanos1 opened this issue May 4, 2019 · 14 comments · Fixed by #474
Closed

Comments

@mjanos1
Copy link

mjanos1 commented May 4, 2019

Hi,

this issue is happen for my project and for clean instalation of webpack created by https://github.com/open-wc/open-wc/tree/master/packages/building-webpack

This happen very randomly, but almost everytime when run build on docker, and in some case when run build under win 10.

"build": "webpack --mode production",

When I try to debug this issue, html-webpack-plugin do correct changes for index.html, but it is not saved in dist/index.html.
What I can observe, modern-and-legacy-config.js randomly start build with legacy build createConfig(options, true) and then with modern build createConfig(options, false), in this case index.html was correctly build.
In modern-and-legacy-config.js is [createConfig(options, false), createConfig(options, true)]

project config:

webpack.config.js:

/* eslint-disable import/no-extraneous-dependencies */

const { resolve } = require('path');
const createDefaultConfig = require('@open-wc/building-webpack/modern-and-legacy-config');
const merge = require('webpack-merge');
const CopyPlugin = require('copy-webpack-plugin');

const defaultConfiguration = createDefaultConfig({
  indexHTML: resolve(__dirname, './src/index.html'),
  entry: resolve(__dirname, './src/index.js'),
});

const [defaultModernConfig, defaultLegacyConfig] = Array.isArray(defaultConfiguration)
  ? defaultConfiguration
  : [defaultConfiguration, defaultConfiguration];

const customConfig = {
  devServer: {
    contentBase: resolve(__dirname, 'dist'),
    historyApiFallback: true,
    https: true,
  },
  plugins: [
    new CopyPlugin([
//copy fonts and css to assets folder
    ]),
  ],
};


module.exports = [
  merge(defaultModernConfig, customConfig),
  merge(defaultLegacyConfig, customConfig),
];

src/index.html:

<!doctype html>
<html lang="en">
<head>
    <meta charset="utf-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0, viewport-fit=cover" />
    <meta name="Description" content="">
    <base href="/">
<!-- Import css files from assets folder -->
</head>
<body>
    <div id="auth"></div>
    <div id="breadcrumbs"></div>
    <div id="root"></div>
    <div id="splash">
        <div class="splash-logo-wrap">
            <svg>
<!-- SVG detail -->
           </svg>
        </div>
    </div>
</body>

</html>

output dist/index.html:

<!doctype html>
<html lang="en">
<head>
    <meta charset="utf-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0, viewport-fit=cover" />
    <meta name="Description" content="">
    <base href="/">
<!-- Import css files from assets folder -->
</head>
<body>
    <div id="auth"></div>
    <div id="breadcrumbs"></div>
    <div id="root"></div>
    <div id="splash">
        <div class="splash-logo-wrap">
            <svg>
<!-- SVG detail -->
           </svg>
        </div>
    </div>
</body>

Dockerfile

FROM node:8.15-alpine as builder

# Install build dependencies
RUN apk update && \
    apk add \
    git \
    python \
    make \
    g++ \
    sassc


# Set unsafe-perm to true to enable running install scripts when running as root
RUN npm config set unsafe-perm true

COPY . /app/

WORKDIR /app

# Install dependencies
RUN npm install

# Start over with a clean image
# Leaves behind build dependencies and SSH keys
FROM node:8.15-alpine as final

RUN apk update && \
    apk add bash

WORKDIR /app

# Copy app dependencies
COPY --from=builder /app/ .

build with node:12.0.0-alpine has same result

@LarsDenBakker
Copy link
Member

This looks to be a race condition of some kind. We need to investigate this. Have you tried this with rollup? Do you see the same issues there?

@mjanos1
Copy link
Author

mjanos1 commented May 6, 2019

@LarsDenBakker thanks for reply.
Initial rollup looks working, also under docker.
I will try to rework my project to rollup config, and let you know.

But is it possible to try fix webpack build?

@LarsDenBakker
Copy link
Member

Indeed, we will look into fixing this. It seems to be a race condition within the webpack hook we use for copying over files.

@LarsDenBakker
Copy link
Member

I am trying to debug this, but I can't reproduce it locally.

@mjanos1
Copy link
Author

mjanos1 commented May 13, 2019

Did you try it with docker?
Or was helpful https://polymer.slack.com/archives/CE6D9DN05/p1557560994221600 ?

@tpluscode
Copy link
Contributor

Hey @LarsDenBakker, any luck with this. If you've got some pointers I'd gladly try help fix this. It's being a pain for us as we need to support IE11 for a client and cannot fulfil that requirement.

@LarsDenBakker
Copy link
Member

I've tried reproducing it in different scenario's, but I haven't been able to yet. I still need to try out your gist, but I didn't have any time last weekend.

@LarsDenBakker
Copy link
Member

LarsDenBakker commented May 25, 2019

This is my output when I try your gist:

<!doctype html>
<html lang="en">
<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width, initial-scale=1.0, viewport-fit=cover" />
  <meta name="Description" content="Put your description here.">

  <style>
    * {
      margin: 0;
      padding: 0;
      font-family: sans-serif;
    }

    body {
      background-color: #ededed;
    }
  </style>
  <title>issue-435</title>
<link rel="preload" href="main.0162c5c97bb253f7cacd.js" as="script"></head>
<body>
  <issue-435></issue-435>

  
<script src="polyfills/babel.js" nomodule></script><script>!function(){function n(n){return new Promise(function(e,o){var t=document.createElement("script");t.onerror=function(){o(new Error("Error loading "+n))},t.onload=function(){e()},t.src=n,t.setAttribute("defer",!0),document.head.appendChild(t)})}var e="noModule"in HTMLScriptElement.prototype?["main.0162c5c97bb253f7cacd.js"]:["legacy/main.1f7bd3bd3e1cd42ed465.js"],o=[];"URLSearchParams"in window||o.push(n("polyfills/url.js")),"attachShadow"in Element.prototype&&"getRootNode"in Element.prototype||o.push(n("polyfills/webcomponents.js")),o.length?Promise.all(o).then(function(){e.forEach(function(e){n(e)})}):e.forEach(function(e){n(e)})}();</script></body>

</html>

which is what I expect.

What version of node are you using? I see that it works correctly on node 8, 10 and 12 but not on node 11 :/

@tpluscode
Copy link
Contributor

I currently have node 10 and I get this:

<!doctype html>
<html lang="en">
<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width, initial-scale=1.0, viewport-fit=cover" />
  <meta name="Description" content="Put your description here.">

  <style>
    * {
      margin: 0;
      padding: 0;
      font-family: sans-serif;
    }

    body {
      background-color: #ededed;
    }
  </style>
  <title>issue-435</title>
</head>
<body>
  <issue-435></issue-435>

  
</body>

</html>

@tpluscode
Copy link
Contributor

tpluscode commented May 25, 2019

But it worked in 8 and 12 while it failed in 10 and 11❗️

@LarsDenBakker
Copy link
Member

That's interesting, in a funky way. What OS are you on?

@tpluscode
Copy link
Contributor

Mac Mojave

@LarsDenBakker
Copy link
Member

I discovered a race condition, I'm wondering if it fixes your issue. Can you try with building-webpack 1.4.5?

@LarsDenBakker
Copy link
Member

I found another issue when terser is in parallel mode. Fed up with it now so im going to rewrite it on the weekend

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 a pull request may close this issue.

3 participants