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

Inconsistent build output wrt "use strict" #3284

Closed
connorjclark opened this issue Aug 3, 2023 · 4 comments
Closed

Inconsistent build output wrt "use strict" #3284

connorjclark opened this issue Aug 3, 2023 · 4 comments

Comments

@connorjclark
Copy link

connorjclark commented Aug 3, 2023

Version 0.18.17, Apple silicon mac

I noticed that my output bundle sometimes differed in its md5 hash. The delta looks like this:

*** dist/variant-905d13ea4519f451db33e1ae22307549.js	Thu Aug  3 16:13:24 2023
--- dist/variant-de0fe63cbb7fbf746e50575f7cfa614e.js	Thu Aug  3 16:14:21 2023
***************
*** 28552,28571 ****
--- 28552,28572 ----
          }
        };
        trace_compat_default = TraceCompat;
      }
    });
  
    // core/computed/processed-navigation.js
    var ProcessedNavigation, ProcessedNavigationComputed;
    var init_processed_navigation = __esm({
      "core/computed/processed-navigation.js"() {
+       "use strict";
        init_process_global();
        init_computed_artifact();
        init_processed_trace();
        init_lh_trace_processor();
        ProcessedNavigation = class {
          static {
            __name(this, "ProcessedNavigation");
          }
          /**
           * @param {LH.Trace | LH.Artifacts.ProcessedTrace} traceOrProcessedTrace
***************
*** 36448,36468 ****
          }
        };
        modern_image_formats_default = ModernImageFormats;
      }
    });
  
    // core/computed/metrics/metric.js
    var Metric, metric_default;
    var init_metric = __esm({
      "core/computed/metrics/metric.js"() {
-       "use strict";
        init_process_global();
        init_trace_processor();
        init_processed_trace();
        init_processed_navigation();
        init_network_records();
        Metric = class {
          static {
            __name(this, "Metric");
          }
          constructor() {
--- 36449,36468 ----

The modules affected always seem to be the same, but a random permuation of them don't include "use strict", resulting in a few different output bundle variants.

Repro

The only reproduction I have is extremely non-trivial. The more I reduced it, the less often the bug occurred. Here's what I ended up wtih:

git clone https://github.com/GoogleChrome/lighthouse.git
cd lighthouse
git checkout esbuild-bug-usestrict
yarn

repeat 10 DEBUG=1 yarn build-devtools &> /dev/null && md5 dist/lighthouse-dt-bundle.js && mv dist/lighthouse-dt-bundle.js dist/variant-$(md5 -q dist/lighthouse-dt-bundle.js).js

# Once you see two hashes...
diff -C 10 dist/variant-*

The entry file is just this:

// Order does not matter
import('./helper.js');
import('./helper2.js');

helper.js:

import * as i18n from 'lighthouse/core/lib/i18n/i18n.js';
import ComputedMetric from 'lighthouse/core/computed/metrics/metric.js';

helper2.js:

import {Audit} from '../../core/audits/audit.js';
import * as i18n from '../../core/lib/i18n/i18n.js';

This is the delta; roughly every 10 runs "use strict" is not present for the i18n.js module.

*** dist/variant-4543d1a84d8b280400f4d8b867a24ba6.js	Thu Aug  3 16:10:29 2023
--- dist/variant-e34faa204baf5ad14a3605915b9d762c.js	Thu Aug  3 16:10:33 2023
***************
*** 3573,3592 ****
--- 3573,3593 ----
          i18nId,
          values,
          formattedDefault: formatMessage(message, values, DEFAULT_LOCALE)
        };
      }, "getIcuMessageFn");
      return getIcuMessageFn;
    }
    var import_lookup_closest_locale, import_meta3, UIStrings;
    var init_i18n = __esm({
      "core/lib/i18n/i18n.js"() {
+       "use strict";
        init_path();
        init_url();
        import_lookup_closest_locale = __toESM(require_lookup_closest_locale(), 1);
        init_lighthouse_logger();
        init_format();
        init_root();
        init_format();
        init_esm_utils();
        import_meta3 = {};
        UIStrings = {

The esbuild invocation is here.


I can attempt to reduce it further if it would assist in debugging. The actual difference seems rather trivial and may not be harmful in practice, so I understand if the best approach is to ignore/close. It was mainly surprising that my bundle did not have the exact same contents every time.

@evanw
Copy link
Owner

evanw commented Aug 5, 2023

Sorry but I can't run your code. Here's what I tried (which was run in GitHub Codespaces):

$ git clone https://github.com/GoogleChrome/lighthouse.git
$ cd lighthouse
$ git checkout esbuild-bug-usestrict
$ yarn
$ DEBUG=1 yarn build-devtools

That gave the following output:

yarn run v1.22.19
$ mkdir -p dist && node ./build/build-bundle.js clients/devtools/devtools-entry.js dist/lighthouse-dt-bundle.js
node:internal/errors:496
    ErrorCaptureStackTrace(err);
    ^

Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'lighthouse' imported from /workspaces/codespaces-blank/lighthouse/node_modules/lighthouse-plugin-publisher-ads/plugin.js
    at new NodeError (node:internal/errors:405:5)
    at packageResolve (node:internal/modules/esm/resolve:781:9)
    at moduleResolve (node:internal/modules/esm/resolve:830:20)
    at defaultResolve (node:internal/modules/esm/resolve:1035:11)
    at DefaultModuleLoader.resolve (node:internal/modules/esm/loader:251:12)
    at DefaultModuleLoader.getModuleJob (node:internal/modules/esm/loader:140:32)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:76:33)
    at link (node:internal/modules/esm/module_job:75:36) {
  code: 'ERR_MODULE_NOT_FOUND'
}

Node.js v20.4.0
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Are there additional commands required to reproduce the issue?

@connorjclark
Copy link
Author

Sorry, I did omit an important repro step. That branch has been updated. The missing step was yarn link && yarn link lighthouse (run within the directory of the cloned repo). It's been added as part of the build-devtools script, so the repro commands remain the same.

I also tried to reproduce without the link feature, using a symlink such that the imports in helper.js above would resolve to the same files as helper2.js (ex: lighthouse/core/lib/i18n/i18n.js -> ./symlink/core/lib/i18n/i18n.js), but that did not reproduce the problem. So either the mechanism by which yarn link resolves modules is a key factor, or the issue cannot occur for relative imports.

@connorjclark
Copy link
Author

connorjclark commented Aug 5, 2023

The repeat command I gave above, which uses md5, is also assuming zsh on OSX. Here is a command that will work in GitHub Codespaces:

zsh # get into a zsh terminal, default is bash on Codespaces
repeat 10 DEBUG=1 yarn build-devtools &> /dev/null && md5sum dist/lighthouse-dt-bundle.js && mv dist/lighthouse-dt-bundle.js dist/variant-$(md5sum dist/lighthouse-dt-bundle.js | awk NF=1).js

@evanw
Copy link
Owner

evanw commented Aug 6, 2023

I looked into this a bit. This happens because core/lib/i18n/i18n.js is imported from both core/lib/lh-error.js and clients/devtools/helper.js and esbuild finds the tsconfig.json file for the import from core/lib/lh-error.js but not for the import from clients/devtools/helper.js. The tsconfig.json file is telling esbuild to insert "use strict" in this case. So the value for tsconfig.json that ends up being respected depends on which import is resolved first, which is non-deterministic due to esbuild's parallelism.

This is an esbuild bug. I'm guessing this is due to symlinks somehow since it happens with the files from yarn link. I'll look into this more later.

@evanw evanw closed this as completed in 63fd6ff Aug 6, 2023
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

No branches or pull requests

2 participants