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

[rollup-plugin-polyfills-loader] Possible race condition while writing polyfile scripts to disk. #2456

Open
kimtaa opened this issue Sep 15, 2023 · 10 comments · May be fixed by #2571
Open

[rollup-plugin-polyfills-loader] Possible race condition while writing polyfile scripts to disk. #2456

kimtaa opened this issue Sep 15, 2023 · 10 comments · May be fixed by #2571

Comments

@kimtaa
Copy link

kimtaa commented Sep 15, 2023

Hi!

I'm having some trouble with getting the polyfill javascript files written to disk. They are only sometimes generated into the polyfills folder - more often if my system is under heavy load. So, I suspect a race condition of some sort.

Below is the config I use:

import { rollupPluginHTML as html } from "@web/rollup-plugin-html";
import {polyfillsLoader} from '@web/rollup-plugin-polyfills-loader';
import {copy} from '@web/rollup-plugin-copy';
import { nodeResolve } from '@rollup/plugin-node-resolve';
import {getBabelOutputPlugin} from '@rollup/plugin-babel';
import terser from '@rollup/plugin-terser';
import pkgMinifyHTML from 'rollup-plugin-minify-html-literals';
const minifyHTML = pkgMinifyHTML.default
import summary from 'rollup-plugin-summary';

const htmlPlugin = html({
  rootDir: './build',
  flattenOutput: false,
  minify: false,
  transformHtml: [
    html => { // add csp nonce template tag as arg to polyfill loader script
      const regex = /(polyfills\.push\(loadScript\()(.*?)(\)\);)/g;
      return html.replace(regex, "$1 $2, null, [{\"name\": \"nonce\",\"value\": \"{{cspNonce}}\"}] $3");
    },
    html => html.replace(/<link rel=/g, "<link nonce=\"{{cspNonce}}\" rel="),
    html => html.replace(/<script>/g, "<script nonce=\"{{cspNonce}}\">")
  ]
});


export default {
  input: 'index.html',
  plugins: [
    htmlPlugin,
    nodeResolve(),
    minifyHTML(),
    terser({
      module: true,
      warnings: true,
    }),
    polyfillsLoader({
      modernOutput: {
        name: 'modern',
      },
      legacyOutput: { name: 'legacy', test: '!(\'noModule\' in HTMLScriptElement.prototype)' },
      polyfills: {
        hash: true,
        coreJs: true,
        regeneratorRuntime: true,
        fetch: true,
        webcomponents: true,
        custom: [
          {
            name: 'lit-polyfill-support',
            path: 'node_modules/lit/polyfill-support.js',
            test: "!('attachShadow' in Element.prototype)",
            module: false,
          },
        ],
      },
    }),
    summary(),
    copy({ rootDir: "build", patterns: "**/*.{svg,ttf,woff,woff2,eot}"}),
  ],
  output: [
    {
      format: 'esm',
      chunkFileNames: '[name]-[hash].js',
      entryFileNames: '[name]-[hash].js',
      dir: 'dist',
      plugins: [htmlPlugin.api.addOutput('modern')],
    },
    {
      preserveModules:false,
      dir: 'dist'
    },
    {
      format: 'esm',
      chunkFileNames: 'legacy-[name]-[hash].js',
      entryFileNames: 'legacy-[name]-[hash].js',
      dir: 'dist',
      plugins: [
        htmlPlugin.api.addOutput('legacy'),
        getBabelOutputPlugin({
          compact: true,
          presets: [
            [
              '@babel/preset-env',
              {
                targets: {
                  ie: '11',
                },
                modules: 'systemjs',
              },
            ],
          ],
        }),
      ],
    },
  ],

};

Any ideas or pointers on how to solve this?

Cheers!

@thepassle
Copy link
Member

Any ideas or pointers on how to solve this?

I recommend creating a minimal reproduction, that usually highlights the issue pretty quickly.

@kimtaa
Copy link
Author

kimtaa commented Sep 16, 2023

Hi again!

Ok - I've created a minimal project, and either I'm hitting the bug all the time, or I've configured something wrong. The 'polyfills'-folder is not created no matter what I try. Could you try this out, and confirm whether or not the polyfills-folder is created for you?

index.html:

<!DOCTYPE html>
<html lang="en">
<head>
  <title>minimal</title>
  <script type="module" src="main-content.js"></script>
</head>
<body>
    <main-content></main-content>
</body>
</html>

main-content.ts:

import {LitElement, html} from 'lit';
import { customElement } from "lit/decorators.js";
@customElement('main-content')
export class MainContent extends LitElement {
  override render()  {
    return html`<p>test output</p>`;
  }
}

package.json:

{
  "name": "mini-test",
  "type": "module",
  "source": "index.html",
  "browserslist": [
    "> 0.1%, last 2 versions"
  ],
  "scripts": {
    "build": "yarn clean && tsc && rollup -c",
    "clean": "rm -rf dist/"
  },
  "dependencies": {
    "@lit-labs/context": "^0.4.0",
    "lit": "^2.2.4"
  },
  "devDependencies": {
    "@babel/core": "^7.0.0",
    "@babel/preset-env": "^7.22.15",
    "@rollup/plugin-babel": "^6.0.3",
    "@rollup/plugin-node-resolve": "^15.2.1",
    "@rollup/plugin-replace": "^5.0.2",
    "@rollup/plugin-terser": "^0.4.3",
    "@types/mocha": "^10.0.1",
    "@typescript-eslint/eslint-plugin": "^5.25.0",
    "@typescript-eslint/parser": "^5.25.0",
    "@web/polyfills-loader": "^2.1.2",
    "@web/rollup-plugin-copy": "^0.4.0",
    "@web/rollup-plugin-html": "^2.0.1",
    "@web/rollup-plugin-polyfills-loader": "^2.0.1",
    "@webcomponents/webcomponentsjs": "^2.6.0",
    "core-js": "^3.32.2",
    "custom-elements-es5-adapter": "^1.0.0",
    "eslint": "^8.15.0",
    "fetch": "^1.1.0",
    "regenerator-runtime": "^0.14.0",
    "rollup": "^3.29.1",
    "rollup-plugin-minify-html-literals": "^1.2.6",
    "rollup-plugin-summary": "^2.0.0",
    "typescript": "~4.7.4",
    "webcomponents": "^0.1.4"
  }
}

rollup.config.js:

import { rollupPluginHTML as html } from "@web/rollup-plugin-html";
import {polyfillsLoader} from '@web/rollup-plugin-polyfills-loader';

const htmlPlugin = html({ input: 'index.html' });
export default {
  onwarn(warning, warn) {
    if (warning.code === 'THIS_IS_UNDEFINED') return;
    warn(warning);
  },
  plugins: [
    htmlPlugin,
    polyfillsLoader({
      modernOutput: {name: 'modern',},
      legacyOutput: { name: 'legacy', test: '!(\'noModule\' in HTMLScriptElement.prototype)' },
      polyfills: {fetch: true},
    }),
  ],
  output: [
    {
      format: 'esm',
      chunkFileNames: '[name]-[hash].js',
      entryFileNames: '[name]-[hash].js',
      dir: './dist',
      plugins: [htmlPlugin.api.addOutput('modern')]
    },
    {
      format: 'esm',
      chunkFileNames: 'legacy-[name]-[hash].js',
      entryFileNames: 'legacy-[name]-[hash].js',
      dir: './dist',
      plugins: [htmlPlugin.api.addOutput('legacy')]
    },
  ],
};

tsconfig.json:

{
  "compilerOptions": {
    "target": "es2019",
    "module": "es2020",
    "lib": ["es2020", "DOM", "DOM.Iterable"],
    "declaration": true,
    "declarationMap": true,
    "sourceMap": true,
    "inlineSources": true,
    "outDir": "./",
    "rootDir": "./",
    "strict": true,
    "noUnusedLocals": true,
    "noUnusedParameters": true,
    "noImplicitReturns": true,
    "noFallthroughCasesInSwitch": true,
    "noImplicitAny": true,
    "noImplicitThis": true,
    "moduleResolution": "node",
    "allowSyntheticDefaultImports": true,
    "experimentalDecorators": true,
    "forceConsistentCasingInFileNames": true,
    "noImplicitOverride": true,
    "allowJs": true,
    "isolatedModules": true,
    "plugins": [
      {
        "name": "ts-lit-plugin",
        "strict": true
      }
    ],
    "types": ["mocha"]
  },
  "include": ["*.ts"],
  "exclude": []
}

I was expecting yarn install && tsc && rollup -c to produce a 'polyfills' folder in 'dist'.

Cheers!

@thepassle
Copy link
Member

There still seems to be quite a lot going on in the rollup config, can you try reducing that even more?

@kimtaa
Copy link
Author

kimtaa commented Sep 16, 2023

Yes - could've been smaller. I've updated the rollup config in the previous reply so that it's more basic now.

@thepassle
Copy link
Member

We set generatedFiles in this callback that is being added to htmlPlugin.api.addHtmlTransformer:

When using multiple outputs, I see that in the generateBundle hook, generatedFiles is undefined:

I tried to wrap the whole generateBundle in a setTimeout just for debugging purposes, and then I see that generatedFiles is defined:

async generateBundle(_, bundle) {
          setTimeout(() => {
            if (generatedFiles) {
              for (const file of generatedFiles) {
                // if the polyfills loader is used multiple times, this polyfill might already be output
                // so we guard against that. polyfills are already hashed, so there is no need to worry
                // about clashing
                if (!(file.path in bundle)) {
                  this.emitFile({
                    type: 'asset',
                    name: file.path,
                    fileName: file.path,
                    source: file.content,
                  });
                }
              }
            }
          })

        },

However, I still don't see them being output to dist/polyfills, even though I see the calls to this.emitFile happening. Im not really sure why 🤔

@kimtaa
Copy link
Author

kimtaa commented Sep 16, 2023

It appears as the html transformer is called after the generateBundle hook?

@kimtaa
Copy link
Author

kimtaa commented Sep 18, 2023

I've tried to follow the source code to understand what is happening.

Looks like the html-transformer that creates generatedFiles (added at rollupPluginPolyfillsLoader.buildStart) is called by the html-plugin as part of the output plugin. So, generatedFiles will always be empty when rollupPluginPolyfillsLoader.generateBundle is called.

Maybe the html-transformer could emit the files if the number of bundles are more than 1?

@aigan
Copy link

aigan commented Dec 4, 2023

I created a test based on the ones in rollup-plugin-html.test.ts.
Modules should keep their order. The tests are done without \n but I include them here for readability

<h1>Hello world</h1>
<script type="module">import "./entrypoint-a.js";</script>
<script type="module" src="./entrypoint-b.js"></script>

should output

<html><head></head><body><h1>Hello world</h1>
<script type="module" src="./inline-module-5ec680a4efbb48ae254268ab1defe610.js"></script>
<script type="module" src="./entrypoint-b.js"></script>
</body></html>

For multiple src there are a race condition, giving random order.
I hope that this example with inline modules are related to that.

@aigan
Copy link

aigan commented Dec 4, 2023

Here is the actual test i wrote for rollup-plugin-html.test.ts

  it('can resolve modules in original order', async () => {
    const config = {
      plugins: [
        rollupPluginHTML({
          rootDir,
          input: {
            name: 'index.html',
            html: '<h1>Hello world</h1>' +
              '<script type="module">import "./entrypoint-a.js";</script>' +
              '<script type="module" src="./entrypoint-b.js"></script>',
          },
        }),
      ],
    };

    const bundle = await rollup(config);
    const { output } = await bundle.generate(outputConfig);
    expect(output.length).to.equal(4);
    const hash = '5ec680a4efbb48ae254268ab1defe610';
    const { code: appCode } = getChunk(output, `inline-module-${hash}.js`);
    expect(appCode).to.include("console.log('entrypoint-a.js');");
    expect(stripNewlines(getAsset(output, 'index.html').source)).to.equal(
      '<html><head></head><body><h1>Hello world</h1>' +
        '<script type="module" src="./inline-module-5ec680a4efbb48ae254268ab1defe610.js"></script>' +
        '<script type="module" src="./entrypoint-b.js"></script>' +
        '</body></html>',
    );
  });

@aigan
Copy link

aigan commented Dec 5, 2023

found the problem. Working on a patch.

@aigan aigan linked a pull request Dec 5, 2023 that will close this issue
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