fix: insert user html more consistently (#42195)

* fix: use an iframe to preserve head and body

* fix: remove unnecessary parsing of html

The contents gets inserted into the DOM during transformHtml, which
is always part of the build pipeline

* fix: pipe contents through iframe

* refactor: use the same code for both transforms

* fix: try to handle test errors better

Co-authored-by: moT01 <20648924+moT01@users.noreply.github.com>
This commit is contained in:
Oliver Eyton-Williams
2022-01-24 19:02:25 +01:00
committed by GitHub
parent 76529a17ba
commit b1fb6adc39
7 changed files with 92 additions and 62 deletions

View File

@ -14,13 +14,6 @@ import {
transformContents transformContents
} from '../../../../../utils/polyvinyl'; } from '../../../../../utils/polyvinyl';
const defaultTemplate = ({ source }) => {
return `
<body id='display-body'>
${source}
</body>`;
};
const wrapInScript = partial( const wrapInScript = partial(
transformContents, transformContents,
content => `<script>${content}</script>` content => `<script>${content}</script>`
@ -67,7 +60,7 @@ export function concatHtml({
template, template,
challengeFiles = [] challengeFiles = []
} = {}) { } = {}) {
const createBody = template ? _template(template) : defaultTemplate; const embedSource = template ? _template(template) : ({ source }) => source;
const head = required const head = required
.map(({ link, src }) => { .map(({ link, src }) => {
if (link && src) { if (link && src) {
@ -99,5 +92,5 @@ A required file can not have both a src and a link: src = ${src}, link = ${link}
} }
}, ''); }, '');
return `<head>${head}</head>${createBody({ source })}`; return `<head>${head}</head>${embedSource({ source })}`;
} }

View File

@ -180,10 +180,12 @@ function getBabelOptions({ preview = false, protect = true }) {
} }
const sassWorker = createWorker(sassCompile); const sassWorker = createWorker(sassCompile);
async function transformSASS(element) { async function transformSASS(contentDocument) {
// we only teach scss syntax, not sass. Also the compiler does not seem to be // we only teach scss syntax, not sass. Also the compiler does not seem to be
// able to deal with sass. // able to deal with sass.
const styleTags = element.querySelectorAll('style[type~="text/scss"]'); const styleTags = contentDocument.querySelectorAll(
'style[type~="text/scss"]'
);
await Promise.all( await Promise.all(
[].map.call(styleTags, async style => { [].map.call(styleTags, async style => {
style.type = 'text/css'; style.type = 'text/css';
@ -192,10 +194,10 @@ async function transformSASS(element) {
); );
} }
async function transformScript(element) { async function transformScript(contentDocument) {
await loadBabel(); await loadBabel();
await loadPresetEnv(); await loadPresetEnv();
const scriptTags = element.querySelectorAll('script'); const scriptTags = contentDocument.querySelectorAll('script');
scriptTags.forEach(script => { scriptTags.forEach(script => {
script.innerHTML = tryTransform(babelTransformCode(babelOptionsJS))( script.innerHTML = tryTransform(babelTransformCode(babelOptionsJS))(
script.innerHTML script.innerHTML
@ -203,59 +205,86 @@ async function transformScript(element) {
}); });
} }
const transformHtml = async function (file) {
const div = document.createElement('div');
div.innerHTML = file.contents;
await Promise.all([transformSASS(div), transformScript(div)]);
return transformContents(() => div.innerHTML, file);
};
// Find if the base html refers to the css or js files and record if they do. If // Find if the base html refers to the css or js files and record if they do. If
// the link or script exists we remove those elements since those files don't // the link or script exists we remove those elements since those files don't
// exist on the site, only in the editor // exist on the site, only in the editor
const transformIncludes = async function (fileP) { const addImportedFiles = async function (fileP) {
const file = await fileP; const file = await fileP;
const div = document.createElement('div'); const transform = frame => {
div.innerHTML = file.contents; const documentElement = frame.contentDocument.documentElement;
const link = const link =
div.querySelector('link[href="styles.css"]') ?? documentElement.querySelector('link[href="styles.css"]') ??
div.querySelector('link[href="./styles.css"]'); documentElement.querySelector('link[href="./styles.css"]');
const script = const script =
div.querySelector('script[src="script.js"]') ?? documentElement.querySelector('script[src="script.js"]') ??
div.querySelector('script[src="./script.js"]'); documentElement.querySelector('script[src="./script.js"]');
const importedFiles = []; const importedFiles = [];
if (link) { if (link) {
importedFiles.push('styles.css'); importedFiles.push('styles.css');
link.remove(); link.remove();
} }
if (script) { if (script) {
importedFiles.push('script.js'); importedFiles.push('script.js');
script.remove(); script.remove();
} }
return {
contents: documentElement.innerHTML,
importedFiles
};
};
const { importedFiles, contents } = await transformWithFrame(
transform,
file.contents
);
return flow( return flow(
partial(setImportedFiles, importedFiles), partial(setImportedFiles, importedFiles),
partial(transformContents, () => div.innerHTML) partial(transformContents, () => contents)
)(file); )(file);
}; };
export const composeHTML = cond([ const transformWithFrame = async function (transform, contents) {
[ // we use iframe here since file.contents is destined to be be inserted into
testHTML, // the root of an iframe.
flow( const frame = document.createElement('iframe');
partial(transformHeadTailAndContents, source => { frame.style = 'display: none';
const div = document.createElement('div'); let out = { contents };
div.innerHTML = source; try {
return div.innerHTML; // the frame needs to be inserted into the document to create the html
}), // element
partial(compileHeadTail, '') document.body.appendChild(frame);
) // replace the root element with user code
], frame.contentDocument.documentElement.innerHTML = contents;
// grab the contents now, in case the transformation fails
out = { contents: frame.contentDocument.documentElement.innerHTML };
out = await transform(frame);
} finally {
document.body.removeChild(frame);
}
return out;
};
const transformHtml = async function (file) {
const transform = async frame => {
await Promise.all([
transformSASS(frame.contentDocument),
transformScript(frame.contentDocument)
]);
return { contents: frame.contentDocument.documentElement.innerHTML };
};
const { contents } = await transformWithFrame(transform, file.contents);
return transformContents(() => contents, file);
};
const composeHTML = cond([
[testHTML, partial(compileHeadTail, '')],
[stubTrue, identity] [stubTrue, identity]
]); ]);
export const htmlTransformer = cond([ const htmlTransformer = cond([
[testHTML, flow(transformHtml, transformIncludes)], [testHTML, flow(transformHtml, addImportedFiles)],
[stubTrue, identity] [stubTrue, identity]
]); ]);

View File

@ -19,13 +19,13 @@ You should create a `meta` element within the `head` element.
```js ```js
// TODO: Once builder is fixed so head info is not in body // TODO: Once builder is fixed so head info is not in body
assert.exists(document.querySelector('body > meta')); assert.exists(document.querySelector('head > meta'));
``` ```
You should give the `meta` tag a `charset` of `UTF-8`. You should give the `meta` tag a `charset` of `UTF-8`.
```js ```js
assert.equal(document.querySelector('body > meta')?.getAttribute('charset'), 'UTF-8'); assert.equal(document.querySelector('head > meta')?.getAttribute('charset'), 'UTF-8');
``` ```
# --seed-- # --seed--

View File

@ -16,20 +16,20 @@ Add a `viewport` definition with a `content` attribute detailing the `width` and
You should create another `meta` element in the `head`. You should create another `meta` element in the `head`.
```js ```js
assert.equal(document.querySelectorAll('body > meta')?.length, 2); assert.equal(document.querySelectorAll('head > meta')?.length, 2);
``` ```
You should give the `meta` a `name` attribute of `viewport`. You should give the `meta` a `name` attribute of `viewport`.
```js ```js
assert.equal(document.querySelectorAll('body > meta[name="viewport"]')?.length, 1); assert.equal(document.querySelectorAll('head > meta[name="viewport"]')?.length, 1);
``` ```
You should give the `meta` a `content` attribute of `width=device-width, initial-scale=1`. You should give the `meta` a `content` attribute of `width=device-width, initial-scale=1`.
```js ```js
// TODO: Double-check this is the only correct answer // TODO: Double-check this is the only correct answer
assert.equal(document.querySelectorAll('body > meta[content="width=device-width, initial-scale=1.0"]')?.length || document.querySelectorAll('body > meta[content="width=device-width, initial-scale=1"]')?.length, 1); assert.equal(document.querySelectorAll('head > meta[content="width=device-width, initial-scale=1.0"]')?.length || document.querySelectorAll('body > meta[content="width=device-width, initial-scale=1"]')?.length, 1);
``` ```
# --seed-- # --seed--

View File

@ -17,19 +17,19 @@ You should add a `title` element to the `head`.
```js ```js
// TODO: Fix once builder puts head in the right place // TODO: Fix once builder puts head in the right place
assert.exists(document.querySelector('body > title')); assert.exists(document.querySelector('head > title'));
``` ```
You should not make the `title` longer than 60 characters. You should not make the `title` longer than 60 characters.
```js ```js
assert.isAtMost(document.querySelector('body > title')?.textContent?.length, 60); assert.isAtMost(document.querySelector('head > title')?.textContent?.length, 60);
``` ```
Try being more descriptive with your `title` element. _Hint: At least 20 characters_ Try being more descriptive with your `title` element. _Hint: At least 20 characters_
```js ```js
assert.isAtLeast(document.querySelector('body > title')?.textContent?.length, 20); assert.isAtLeast(document.querySelector('head > title')?.textContent?.length, 20);
``` ```
# --seed-- # --seed--

View File

@ -25,7 +25,7 @@
"delete-step": "cross-env CALLING_DIR=$INIT_CWD node ../tools/challenge-helper-scripts/delete-step", "delete-step": "cross-env CALLING_DIR=$INIT_CWD node ../tools/challenge-helper-scripts/delete-step",
"lint": "gulp lint", "lint": "gulp lint",
"reorder-steps": "cross-env CALLING_DIR=$INIT_CWD node ../tools/challenge-helper-scripts/reorder-steps", "reorder-steps": "cross-env CALLING_DIR=$INIT_CWD node ../tools/challenge-helper-scripts/reorder-steps",
"test": "mocha --delay --exit --reporter progress --bail", "test": "mocha --delay --exit --reporter progress --bail",
"test:full-output": "cross-env FULL_OUTPUT=true mocha --delay --reporter progress" "test:full-output": "cross-env FULL_OUTPUT=true mocha --delay --reporter progress"
}, },
"devDependencies": { "devDependencies": {

View File

@ -73,6 +73,14 @@ const { flatten, isEmpty, cloneDeep, isEqual } = lodash;
// rethrow unhandled rejections to make sure the tests exit with -1 // rethrow unhandled rejections to make sure the tests exit with -1
process.on('unhandledRejection', err => handleRejection(err)); process.on('unhandledRejection', err => handleRejection(err));
// If an uncaught exception gets here, then mocha is in an unexpected state. All
// we can do is log the exception and exit with a non-zero code.
process.on('uncaughtException', err => {
console.error('Uncaught exception:', err.message);
console.error(err.stack);
// eslint-disable-next-line no-process-exit
process.exit(1);
});
const handleRejection = err => { const handleRejection = err => {
// setting the error code because node does not (yet) exit with a non-zero // setting the error code because node does not (yet) exit with a non-zero