fix(hydrate): output track elements as void elms #5720

rwaskiewicz · 2024-04-30T15:27:37Z

What is the current behavior?

See 'new behavior' section

GitHub Issue Number: Fixes: #2994

What is the new behavior?

add track to the list of "EMPTY" elements (void elements) that is referenced when we serialize nodes.

prior to this fix, the output of running renderToString as such:

const hydrate = require('./hydrate/index.js');

const src = `<video>
<source src="x" type="video/mp4">
<track
    kind="subtitles"
    src="x"
>
</video>`;

hydrate.renderToString(src, { prettyHtml: true })
    .then(result => console.log(result.html));

would result in the following being printed:

<!doctype html>
<html class="hydrated" data-stencil-build="shc9pw4e">
  <head>
    <meta charset="utf-8">
  </head>
  <body>
    <video>
      <source src="x" type="video/mp4">
      <!-- NOTE: track has a closing tag -->
      <track kind="subtitles" src="x"></track>
    </video>
  </body>
</html>

With this fix applied, we print track as a void element:

<!doctype html>
<html class="hydrated" data-stencil-build="z6oqy2b2">
  <head>
    <meta charset="utf-8">
  </head>
  <body>
    <video>
      <source src="x" type="video/mp4">
      <track kind="subtitles" src="x">
    </video>
  </body>
</html>

Documentation

N/A

Does this introduce a breaking change?

Yes
No

Testing

First, I spun up a little stencil component starter locally:

$ npm init stencil@latest component track-test && \
  cd $_ && \
  npm i && \
  git add . && \
  git commit -m 'commit package-lock'

Note - this just adds some boilerplate. We won't use (much) of this code.

Next, add the dist-hydrate to your stencil.config.ts

diff --git a/stencil.config.ts b/stencil.config.ts
index 0d78965..09a0a24 100644
--- a/stencil.config.ts
+++ b/stencil.config.ts
@@ -19,6 +19,9 @@ export const config: Config = {
       type: 'www',
       serviceWorker: null, // disable service workers
     },
+    {
+      type: 'dist-hydrate-script',
+    }
   ],
   testing: {
     browserHeadless: "new",

Run npm run build to generate the hydrate output.
Create the following index.js file at the root of the project:

const hydrate = require('./hydrate/index.js');

const src = `<video>
<source src="x" type="video/mp4">
<track
    kind="subtitles"
    src="x"
>
</video>`;

hydrate.renderToString(src, { prettyHtml: true })
    .then(result => console.log(result.html));

Run node index.js, observe the output:

<!doctype html>
<html class="hydrated" data-stencil-build="shc9pw4e">
  <head>
    <meta charset="utf-8">
  </head>
  <body>
    <video>
      <source src="x" type="video/mp4">
      <track kind="subtitles" src="x"></track>
    </video>
  </body>
</html>

Checkout this branch, build it, and install it into your project
Run npm run build again to regenerate the hydrate bundle
Re-run node index.js, observe the difference in output:

<!doctype html>
<html class="hydrated" data-stencil-build="z6oqy2b2">
  <head>
    <meta charset="utf-8">
  </head>
  <body>
    <video>
      <source src="x" type="video/mp4">
-      <track kind="subtitles" src="x"></track>
+      <track kind="subtitles" src="x">
    </video>
  </body>
</html>

Other information

STENCIL-94: Using hydrate.renderToString produces "invalid" HTML with

add `track` to the list of "EMPTY" elements (void elements) that is referenced when we serialize nodes. prior to this fix, the output of running `renderToString` as such: ```js const hydrate = require('./hydrate/index.js'); const src = `<video> <source src="x" type="video/mp4"> <track kind="subtitles" src="x" > </video>`; hydrate.renderToString(src, { prettyHtml: true }) .then(result => console.log(result.html)); ``` would result in the following being printed: ```html <!doctype html> <html class="hydrated" data-stencil-build="shc9pw4e"> <head> <meta charset="utf-8"> </head> <body> <video> <source src="x" type="video/mp4">  <track kind="subtitles" src="x"></track> </video> </body> </html> ``` With this fix applied, we print `track` as a void element: ```html <!doctype html> <html class="hydrated" data-stencil-build="z6oqy2b2"> <head> <meta charset="utf-8"> </head> <body> <video> <source src="x" type="video/mp4"> <track kind="subtitles" src="x"> </video> </body> </html> ``` Fixes: #2994 STENCIL-94: Using hydrate.renderToString produces "invalid" HTML with <track>

github-actions · 2024-04-30T15:28:58Z

`--strictNullChecks` error report

Typechecking with --strictNullChecks resulted in 1138 errors on this branch.

That's the same number of errors on main, so at least we're not creating new ones!

reports and statistics

Our most error-prone files

Path	Error Count
src/dev-server/index.ts	37
src/dev-server/server-process.ts	32
src/compiler/prerender/prerender-main.ts	22
src/runtime/client-hydrate.ts	20
src/testing/puppeteer/puppeteer-element.ts	20
src/screenshot/connector-base.ts	19
src/runtime/vdom/vdom-render.ts	17
src/dev-server/request-handler.ts	15
src/compiler/prerender/prerender-optimize.ts	14
src/compiler/sys/stencil-sys.ts	14
src/sys/node/node-sys.ts	14
src/compiler/prerender/prerender-queue.ts	13
src/compiler/sys/in-memory-fs.ts	13
src/runtime/connected-callback.ts	13
src/runtime/set-value.ts	13
src/compiler/output-targets/output-www.ts	12
src/compiler/transformers/test/parse-vdom.spec.ts	12
src/compiler/transformers/transform-utils.ts	12
src/compiler/transpile/transpile-module.ts	12
src/mock-doc/test/attribute.spec.ts	12

Our most common errors

Typescript Error Code	Count
TS2322	361
TS2345	344
TS18048	205
TS18047	82
TS2722	37
TS2532	24
TS2531	21
TS2454	14
TS2790	11
TS2352	9
TS2769	8
TS2538	8
TS2416	7
TS2493	3
TS18046	2
TS2684	1
TS2430	1

Unused exports report

There are 15 unused exports on this PR. That's the same number of errors on main, so at least we're not creating new ones!

Unused exports

File	Line	Identifier
src/runtime/bootstrap-lazy.ts	21	setNonce
src/screenshot/screenshot-fs.ts	18	readScreenshotData
src/testing/testing-utils.ts	198	withSilentWarn
src/utils/index.ts	145	CUSTOM
src/utils/index.ts	245	NODE_TYPES
src/utils/index.ts	269	normalize
src/utils/index.ts	7	escapeRegExpSpecialCharacters
src/compiler/app-core/app-data.ts	25	BUILD
src/compiler/app-core/app-data.ts	115	Env
src/compiler/app-core/app-data.ts	117	NAMESPACE
src/compiler/fs-watch/fs-watch-rebuild.ts	123	updateCacheFromRebuild
src/compiler/types/validate-primary-package-output-target.ts	61	satisfies
src/compiler/types/validate-primary-package-output-target.ts	61	Record
src/testing/puppeteer/puppeteer-declarations.ts	485	WaitForEventOptions
src/compiler/sys/fetch/write-fetch-success.ts	7	writeFetchSuccessSync

github-actions · 2024-04-30T15:29:26Z

PR built and packed!

Download the tarball here: https://github.com/ionic-team/stencil/actions/runs/8899161346/artifacts/1461462404

If your browser saves files to ~/Downloads you can install it like so:

unzip -d ~/Downloads ~/Downloads/stencil-core-4.17.2-dev.1714498389.9c14110.tgz.zip && npm install ~/Downloads/stencil-core-4.17.2-dev.1714498389.9c14110.tgz

christian-bromann

LGTM 👍

I was thinking how much value a unit test would have where we just stringify all these elements (e.g. in src/mock-doc/test/serialize-node.spec.ts) and snapshot the result verifying that they have no end tag.

rwaskiewicz · 2024-04-30T17:32:55Z

@christian-bromann updated with tests in c02178a

christian-bromann

Awesome 👍

rwaskiewicz force-pushed the rw/fix-track-elm branch from b04a0ce to 97a1ce6 Compare April 30, 2024 15:27

rwaskiewicz marked this pull request as ready for review April 30, 2024 15:45

rwaskiewicz requested a review from a team as a code owner April 30, 2024 15:45

rwaskiewicz requested review from christian-bromann and tanner-reits April 30, 2024 15:45

christian-bromann approved these changes Apr 30, 2024

View reviewed changes

review(cb): unit tests

c02178a

rwaskiewicz requested a review from christian-bromann April 30, 2024 17:32

christian-bromann approved these changes Apr 30, 2024

View reviewed changes

tanner-reits approved these changes Apr 30, 2024

View reviewed changes

rwaskiewicz added this pull request to the merge queue Apr 30, 2024

Merged via the queue into main with commit 2082351 Apr 30, 2024
129 checks passed

rwaskiewicz deleted the rw/fix-track-elm branch April 30, 2024 20:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(hydrate): output track elements as void elms #5720

fix(hydrate): output track elements as void elms #5720

rwaskiewicz commented Apr 30, 2024

github-actions bot commented Apr 30, 2024

github-actions bot commented Apr 30, 2024 •

edited

christian-bromann left a comment

rwaskiewicz commented Apr 30, 2024

christian-bromann left a comment

fix(hydrate): output track elements as void elms #5720

fix(hydrate): output track elements as void elms #5720

Conversation

rwaskiewicz commented Apr 30, 2024

What is the current behavior?

What is the new behavior?

Documentation

Does this introduce a breaking change?

Testing

Other information

github-actions bot commented Apr 30, 2024

--strictNullChecks error report

reports and statistics

Unused exports report

github-actions bot commented Apr 30, 2024 • edited

PR built and packed!

christian-bromann left a comment

Choose a reason for hiding this comment

rwaskiewicz commented Apr 30, 2024

christian-bromann left a comment

Choose a reason for hiding this comment

`--strictNullChecks` error report

github-actions bot commented Apr 30, 2024 •

edited