live reload broken (duplicate scripts like vendor.js appear)

[jacobq]

Reproduction: https://github.com/jacobq/embroider-live-reload-bug-repro

Enabling embroider in minimal (ember-cli v4.2.0) app breaks live reload feature. When ember s first gets run things are OK. However, changing a file (triggering update/reload) causes something weird to happen such that multiple vendor.js scripts appear (one gets added after each change/reload).

Compare link for v1.6.0 vs v1.7.1 (known good / bad):
v1.6.0...v1.7.1

[bertdeblock]

Seeing this issue as well starting from v1.7.0. Downgrading to v1.6.0 seems to resolve it for now.

[bwbuchanan]

Also seeing this issue. I narrowed it down to @embroider/webpack. Pinning that package to 1.6.0 resolves the problem.

[ef4]

Most likely culprit is #1177.

[krisselden]

I'm taking a look at this, thank you for the reproduction.

[muziejus]

This error caught me yesterday, but I wanted to report a side effect. I don't know a lot about how yarn's internals work, but I had two apps, one with the embroider addons at ^1.7.1 and one at ^1.5.0. As soon as the problem expressed itself in the former, it started expressing itself in the latter, too, and no amount of deleting node_modules, etc., fixed it. I finally pinned both apps at 1.6.0 (no caret), and now both seem to be doing ok.

[jacobq]

This error caught me yesterday, but I wanted to report a side effect. I don't know a lot about how yarn's internals work, but I had two apps, one with the embroider addons at ^1.7.1 and one at ^1.5.0. As soon as the problem expressed itself in the former, it started expressing itself in the latter, too, and no amount of deleting node_modules, etc., fixed it. I finally pinned both apps at 1.6.0 (no caret), and now both seem to be doing ok.

It seems quite possible to me that the problem is caused by an upstream dependency. #1208 was a sort of "mass update" (assuming semver contract is respected and no private APIs are being used then this shouldn't break anything, but in practice it often does, especially when many packages are upgraded), and though I haven't looked into it, my first hunch would be to try pinning livereload-js back to 3.3.3 instead of 3.4.0.

• livereload-js
  • upgrade
  • ~~v3.3.3...v3.4.0 diff
• terser
  • upgrade
  • changelog
  • v5.10.0...v5.13.1 diff

Edit: I tried pinning livereload-js and terser, but it didn't seem to help.

Edit: As ef4 noted below, I was mistaken; this lock file should not affect consuming apps/addons.

[ef4]

@jacobq #1208 should only effect embroider's own test suite. The yarn.lock here doesn't have any impact on apps that use embroider.

[jacobq]

Bisection

Looks like bug first occurs in commit 23b8b2f (b59a578 is OK). Hope that helps.

`git bisect log`

# bad: [c95b3a1f64dcb149628d30c45da61bf2a43652c9] Release 1.7.0
# good: [d60d0b95c64ee73a44777a975b10f6708402ee26] Release 1.6.0
git bisect start 'v1.7.0' 'v1.6.0'
# good: [b59a5780a0b177fc14568f943bac9b1808201c79] Merge pull request #1194 from embroider-build/adjust-webpack-output-dir
git bisect good b59a5780a0b177fc14568f943bac9b1808201c79
# bad: [c757450917e597335407fa38645dae70c1e9b138] Merge pull request #1201 from angelayanpan/patch-1
git bisect bad c757450917e597335407fa38645dae70c1e9b138
# bad: [0ea842ed577e5bbf655d64bfb13159c2db06d5f7] Fix double tags when writing html entrypoint twice.
git bisect bad 0ea842ed577e5bbf655d64bfb13159c2db06d5f7
# bad: [23b8b2f6670bf314aefd7c2a295bd5f39f1a916c] make write files a plugin
git bisect bad 23b8b2f6670bf314aefd7c2a295bd5f39f1a916c
# first bad commit: [23b8b2f6670bf314aefd7c2a295bd5f39f1a916c] make write files a plugin
# bad: [23b8b2f6670bf314aefd7c2a295bd5f39f1a916c] make write files a plugin
git bisect bad 23b8b2f6670bf314aefd7c2a295bd5f39f1a916c
# first bad commit: [23b8b2f6670bf314aefd7c2a295bd5f39f1a916c] make write files a plugin

Process followed for bisection

1. Setup embroider monorepo

   1. git clone git@github.com:embroider-build/embroider.git
   2. cd embroider
   3. yarn
   4. cd packages
   5. For each of core, compat, and webpack folders:
      • cd <folder>
      • yarn link
      • cd ..

2. In the clone of the reproduction repo listed above: yarn link @embroider/compat @embroider/core @embroider/webpack

3. In embroider root: git bisect start v1.7.0 v1.6.0

4. For each step in the bisection:

   1. Run yarn in the embroider repo
   2. Run ember s in the reproduction repo
   3. Run git bisect good or git bisect bad in the embroider repo after testing if the bug is present
   4. Stop running ember s

[jacobq]

Commenting out these 4 lines makes the problem go away:

embroider/packages/webpack/src/ember-webpack.ts

Lines 280 to 283 in 7099a11

	if (this.lastWebpack && this.lastAppInfo && equalAppInfo(appInfo, this.lastAppInfo)) {
	debug(`reusing webpack config`);
	return this.lastWebpack;
	}

Maybe somehow we're re-adding the changed files each time?

[jacobq]

Maybe somehow we're re-adding the changed files each time?

I think I figured it out: when we cache/reuse the webpack instance (i.e. this.lastWebpack) we do not pass it the newly created HTMLEntrypoints (in appInfo), so the JSDOM instances held by the existing/previous webpack instance are getting used again (keeping the previously inserted content and then adding it again). The equalAppInfo doesn't inspect the state of the entrypoints' JSDOM instances

[jacobq]

In case it's helpful for anyone, here's a little script to link/unlink each embroider package (not sure if there's an easier way to do this):

yarn-link-or-unlink.cjs

// yarn-link-or-unlink.cjs
// (assumes __dirname is root of embroider repo)

const { execSync } = require('child_process');
const { readdirSync } = require('fs');
const { argv } = require('process');
const path = require('path');
// change this if you put this script somewhere besides the root of the embroider repo
const embroiderPackagesDir = path.join(__dirname, 'packages'); 

function log(...args) { console.log(...args); }

const action = argv[2] === 'unlink' ? 'unlink' : 'link';
log(`action = ${action}`);

const packages = readdirSync(embroiderPackagesDir);
log('packages -->', packages);

for (const packageName of packages) {
  const packagePath = path.join(embroiderPackagesDir, packageName);
  log(`${action}ing ${packageName} (in ${packagePath})`);
  execSync(`yarn ${action}`, { cwd: packagePath, encoding: 'utf-8' });
}

const packageListString = packages.map((name) => `@embroider/${name}`).join(' ');
log(`\nRun 'yarn ${action} ${packageListString}' in consuming project to update it`);