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-html] Incorrect <script type="module"> order #2466

Open
jdcoldsmith opened this issue Sep 27, 2023 · 1 comment · May be fixed by #2571
Open

[rollup-plugin-html] Incorrect <script type="module"> order #2466

jdcoldsmith opened this issue Sep 27, 2023 · 1 comment · May be fixed by #2571

Comments

@jdcoldsmith
Copy link

Versions

@web/rollup-plugin-html: 2.0.1
rollup: 3.29.3

Plugin config

// plugin for generating HTML files from rollup
    html({
      // [not working yet] don't build mobile (we have another rollup.config.mobile.js for that)
      exclude: [ 'mobile/**', ...pagesToIgnore ],
      // keep our file structure intact
      flattenOutput: false,
      // the root directory to start at
      rootDir: 'source',
      // keep non-module assets
      extractAssets: true,
      // make whitelabel replacements (if whitelabel exists)
      transformHtml: (html) => {
        if (!!WHITELABEL_NAME) html = html.replace(/Katapult/g, WHITELABEL_NAME);
        return html;
      }
    })

Pre-bundled index.html file

<!DOCTYPE html>
<html>

<head>
  <meta charset="UTF-8">
  <script>
    // Redirect if Mobile
    if (/(mobile|ipad|android|iphone)/i.test(navigator.userAgent)) {
      window.location.replace(window.location.href.replace('/map/', '/mobile/'));
    }
  </script>
  <script src="../../node_modules/web-animations-js/web-animations-next-lite.min.js"></script>
  <script type="module" src="../_resources/app-configuration/app-configuration.js"></script>
  <script type="module" src="../_resources/elements/katapult-maps-desktop/katapult-maps-desktop.js"></script>

  <!-- Material Icons -->
  <link href="https://fonts.googleapis.com/icon?family=Material+Icons" rel="stylesheet">
  <!-- Material Symbols -->
  <link href="https://fonts.googleapis.com/css2?family=Material+Symbols+Rounded:opsz,wght,FILL,GRAD@20..48,100..700,0..1,-50..200" rel="stylesheet"/>
  <!--Favicon Code-->
  <link rel="apple-touch-icon" sizes="180x180" href="https://storage.googleapis.com/katapult-pro-shared-files/photos/favicons/Katapult/apple-touch-icon.png">
  <link rel="icon" type="image/png" sizes="32x32" href="https://storage.googleapis.com/katapult-pro-shared-files/photos/favicons/Katapult/favicon-32x32.png">
  <link rel="icon" type="image/png" sizes="16x16" href="https://storage.googleapis.com/katapult-pro-shared-files/photos/favicons/Katapult/favicon-16x16.png">
  <link rel="mask-icon" href="https://storage.googleapis.com/katapult-pro-shared-files/photos/favicons/Katapult/safari-pinned-tab.svg" color="#003e51">
  <link rel="shortcut icon" href="https://storage.googleapis.com/katapult-pro-shared-files/photos/favicons/Katapult/favicon.ico">
  <meta name="msapplication-config" content="https://storage.googleapis.com/katapult-pro-shared-files/photos/favicons/Katapult/browserconfig.xml">
  <meta name="theme-color" content="#003E51">
  <!--End Favicon Code-->
  

</head>

  <body>
    <katapult-maps-desktop id="katapultMaps"></katapult-maps-desktop>
  </body>

</html>

Bundled index.html file

<!DOCTYPE html><html><head>
  <meta charset="UTF-8">
  <script>
    // Redirect if Mobile
    if (/(mobile|ipad|android|iphone)/i.test(navigator.userAgent)) {
      window.location.replace(window.location.href.replace('/map/', '/mobile/'));
    }
  </script>
  <script src="../assets/web-animations-next-lite.min-79f638b4.js"></script>
  
  

  <!-- Material Icons -->
  <link href="https://fonts.googleapis.com/icon?family=Material+Icons" rel="stylesheet">
  <!-- Material Symbols -->
  <link href="https://fonts.googleapis.com/css2?family=Material+Symbols+Rounded:opsz,wght,FILL,GRAD@20..48,100..700,0..1,-50..200" rel="stylesheet">
  <!--Favicon Code-->
  <link rel="apple-touch-icon" sizes="180x180" href="https://storage.googleapis.com/katapult-pro-shared-files/photos/favicons/Katapult/apple-touch-icon.png">
  <link rel="icon" type="image/png" sizes="32x32" href="https://storage.googleapis.com/katapult-pro-shared-files/photos/favicons/Katapult/favicon-32x32.png">
  <link rel="icon" type="image/png" sizes="16x16" href="https://storage.googleapis.com/katapult-pro-shared-files/photos/favicons/Katapult/favicon-16x16.png">
  <link rel="mask-icon" href="https://storage.googleapis.com/katapult-pro-shared-files/photos/favicons/Katapult/safari-pinned-tab.svg" color="#003e51">
  <link rel="shortcut icon" href="https://storage.googleapis.com/katapult-pro-shared-files/photos/favicons/Katapult/favicon.ico">
  <meta name="msapplication-config" content="https://storage.googleapis.com/katapult-pro-shared-files/photos/favicons/Katapult/browserconfig.xml">
  <meta name="theme-color" content="#003E51">
  <!--End Favicon Code-->
  

</head>

  <body>
    <katapult-maps-desktop id="katapultMaps"></katapult-maps-desktop>
  


<script type="module" src="../katapult-maps-desktop.js"></script><script type="module" src="../app-configuration.js"></script></body></html>

Issue

The problem here is that after being bundled, the page will now execute the ../katapult-maps-desktop.js script before the ../app-configuration.js script which is not the correct order according to the pre-bundled file. This issue came about when upgrading from rollup@2.79.1 to v3. Also interestingly, this only happens to certain pages. We have multiple pages in our app with similar index.html files as this one but it seems random as to which ones build with an incorrect script order and which ones build correctly. Any help would be appreciated!

@jdcoldsmith
Copy link
Author

jdcoldsmith commented Oct 6, 2023

Update! While this still does seem like a bug, my workaround was to import app-configuration.js at the top of katapult-maps-desktop.js instead of it being imported using a script tag.

image

@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.

1 participant