Skip to content

Commit

Permalink
fix(www): ensure that files necessary for www build are on disk
Browse files Browse the repository at this point in the history
This fixes an issue where the `www` build and, in particular, some code
that attempts to inline the entry point for the `www` build into the
`index.html` file that the build outputs, was depending on a file which,
in certain circumstances, would not yet be built. This produced
inconsistency across builds, where the `www` build would pick up and
inline the entry point from the _previous_ build (if the build wasn't
deleted) instead of from the current build.

To go into more detail, prior to this change with the `www` output
target if you did:

```sh
stencil build --dev --watch --serve
```

in a Stencil project and _then_ you ran a build (without deleting
`www/`) like:

```sh
stencil build
```

You'd get a non-working build. Not good!

Why did this happen? We have this function which is called as part of
the `www` build:

https://github.com/ionic-team/stencil/blob/5c4ea3d5354596e5cfb544f91bde7f61de1888b4/src/compiler/html/inline-esm-import.ts#L10-L105

This will basically look for the entry point for the `www` build at
`www/build/project-name.js` and, if it finds it, it will inline that
code in the `index.html` file, reducing the number of JS files that have
to be fetch on page-load by one.

However! Without this change we have a race condition where the lazy
build (which writes the file we're looking for to disk) is not written
until after the `www` build executes. This means that:

- on the first build the entry point is not found, so nothing is inlined
  in the HTML. this produces a working build, but the optimization
  (inlining) is missed.
- on the second build, if `www/` is not deleted then the entry point
  from the _first_ build is found and inlined _or_ if the file is larger
  than a cutoff size then a hashed copy of it will be made and the
  `<script>` tag in `index.html` will be changed to point to the hashed
  copy.
    - in either case the result is that `index.html` will include the
      entry point _for the previous build_ rather than for the _current_
      build and this can produce broken behavior, in particular because
      this might import _other_ files that are no longer on the disk
      (and also because, you know, if you run a build you expect that
      the build emits code based on the current source code, not the
      source code the last time you ran a build)

Further, if both builds were 1) non-HMR / dev-server builds (i.e. the
result of `stencil build`) then, provided there were no code changes
between build one and build two, a working, inlined build will be the
result. However! If _either_ the first build was an HMR build (i.e. from
`stencil build --dev --watch --serve`) _or_ there was a code change so
the hashed filenames will change then the file that is picked up will
refer to things that don't exist. Not good!

fixes #2687
fixes #4948
closes #2970
  • Loading branch information
alicewriteswrongs committed Oct 27, 2023
1 parent 2c8cfac commit 1bf77e0
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 2 deletions.
6 changes: 5 additions & 1 deletion src/compiler/html/inline-esm-import.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,11 @@ export const optimizeEsmImport = async (
if (results.orgImportPaths.length > 0) {
// insert inline script
script.removeAttribute('src');
script.innerHTML = results.code;
// the `'\n'` is added here to avoid possible issues with HTML parsers
// since the inlined JS will often end in a sourcemap-related `//`
// comment a newline ensures that in the rendered HTML string the
// closing script tag will be on its own line
script.innerHTML = results.code + '\n';
}
} else {
const hashedFile = await generateHashedCopy(config, compilerCtx, entryPath);
Expand Down
7 changes: 6 additions & 1 deletion src/compiler/output-targets/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,14 @@ export const generateOutputTargets = async (
outputHydrateScript(config, compilerCtx, buildCtx),
outputLazyLoader(config, compilerCtx),
outputLazy(config, compilerCtx, buildCtx),
outputWww(config, compilerCtx, buildCtx),
]);

// the www output target depends on the output of the lazy output target
// since it attempts to inline the lazy build entry point into `index.html`
// so we want to ensure that the lazy OT has already completed and written
// all of its files before the www OT runs.
await outputWww(config, compilerCtx, buildCtx);

// must run after all the other outputs
// since it validates files were created
await outputDocs(config, compilerCtx, buildCtx);
Expand Down

0 comments on commit 1bf77e0

Please sign in to comment.