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
Process inline styles and scripts #1456
Changes from 1 commit
f09cfc5
a612b82
63cd6c8
d7e5e85
0480066
611a2ab
a496c4a
0c2bbd5
b5de45f
5524c82
3b5c509
7a6d18c
d6facce
bc2a891
9451aa1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -62,6 +62,11 @@ const META = { | |
] | ||
}; | ||
|
||
const SCRIPT_TYPES = { | ||
'application/javascript': 'js', | ||
'application/json': 'json' | ||
}; | ||
|
||
// Options to be passed to `addURLDependency` for certain tags + attributes | ||
const OPTIONS = { | ||
a: { | ||
|
@@ -160,8 +165,71 @@ class HTMLAsset extends Asset { | |
} | ||
} | ||
|
||
generate() { | ||
return this.isAstDirty ? render(this.ast) : this.contents; | ||
async generate() { | ||
let parts = []; | ||
this.ast.walk(node => { | ||
if (node.tag === 'script' || node.tag === 'style') { | ||
if (node.content) { | ||
let value = node.content.join('').trim(); | ||
if (value) { | ||
parts.push({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a bit too clever for my IQ 😅 |
||
type: | ||
node.tag === 'style' | ||
? 'css' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we also support the type attribute on style elements as well to allow other languages like sass or stylus to be inlined in an HTML file? https://developer.mozilla.org/en-US/docs/Web/HTML/Element/style There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, shouldn't be very hard |
||
: node.attrs && node.attrs.type | ||
? SCRIPT_TYPES[node.attrs.type] || 'js' | ||
: 'js', | ||
value | ||
}); | ||
} | ||
} | ||
} | ||
|
||
if (node.attrs && node.attrs.style) { | ||
parts.push({ | ||
type: 'css', | ||
value: node.attrs.style | ||
}); | ||
} | ||
|
||
return node; | ||
}); | ||
|
||
return parts; | ||
} | ||
|
||
async postProcess(generated) { | ||
// Hacky way of filtering out the css hmr JS | ||
// Related: https://github.com/parcel-bundler/parcel/issues/1389 | ||
generated = generated.filter( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line might cause issues, related to this issue: #1389 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this won't work if HMR is actually enabled - the generated code won't be empty in that case. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also probably need to filter out source maps or they'll mess up the indexing below |
||
generatedAsset => | ||
!(generatedAsset.type === 'js' && generatedAsset.value === '') | ||
); | ||
|
||
// Update processed inlined assets | ||
let index = 0; | ||
this.ast.walk(node => { | ||
if (node.tag === 'script' || node.tag === 'style') { | ||
if (node.content) { | ||
node.content = generated[index].value; | ||
index++; | ||
} | ||
} | ||
|
||
if (node.attrs && node.attrs.style) { | ||
node.attrs.style = generated[index].value; | ||
index++; | ||
} | ||
|
||
return node; | ||
}); | ||
|
||
return [ | ||
{ | ||
type: 'html', | ||
value: render(this.ast) | ||
} | ||
]; | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -277,11 +277,6 @@ describe('html', function() { | |
) | ||
); | ||
|
||
// minifyJson | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why was this removed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because htmlnano is no longer responsible for JSON minifying, although this caused the tests to not show that it actually changed json into JS so I've put it back |
||
assert( | ||
html.includes('<script type="application/json">{"user":"me"}</script>') | ||
); | ||
|
||
// minifySvg is false | ||
assert( | ||
html.includes( | ||
|
@@ -576,4 +571,32 @@ describe('html', function() { | |
] | ||
}); | ||
}); | ||
|
||
it('should process inline JS', async function() { | ||
let b = await bundle(__dirname + '/integration/html-inline-js/index.html', { | ||
production: true | ||
}); | ||
|
||
const bundleContent = (await fs.readFile(b.name)).toString(); | ||
assert(!bundleContent.includes('someArgument')); | ||
}); | ||
|
||
it('should process inline styles', async function() { | ||
let b = await bundle( | ||
__dirname + '/integration/html-inline-styles/index.html', | ||
{production: true} | ||
); | ||
|
||
await assertBundleTree(b, { | ||
name: 'index.html', | ||
assets: ['index.html'], | ||
childBundles: [ | ||
{ | ||
type: 'jpg', | ||
assets: ['img.jpg'], | ||
childBundles: [] | ||
} | ||
] | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
<!DOCTYPE html> | ||
<html lang="en"> | ||
<head> | ||
<title>Inline JavaScript Parcel</title> | ||
</head> | ||
<body> | ||
<script> | ||
var hello = "Hello"; | ||
let getHello = (someArgument) => { | ||
return someArgument; | ||
} | ||
</script> | ||
<script> | ||
var world = "world"; | ||
</script> | ||
<script> | ||
var end = "!"; | ||
</script> | ||
<script> | ||
console.log(`${hello} ${world}${end}`); | ||
</script> | ||
</body> | ||
</html> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
<!DOCTYPE html> | ||
<html lang="en"> | ||
<head> | ||
<title>Inline JavaScript Parcel</title> | ||
</head> | ||
<body> | ||
<style> | ||
.someClass { | ||
background: blue; | ||
color: white; | ||
} | ||
</style> | ||
|
||
<div class="someClass"> | ||
<h1 style="color: green;"></h1> | ||
<p style="background: url(./img.jpg);"></p> | ||
</div> | ||
</body> | ||
</html> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parcel compiles json to javascript currently, so this might actually break things. haven't tested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does, and it does mess up the inlining, might have to look into that