Skip to content
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

Merged
merged 15 commits into from Jul 21, 2018
6 changes: 3 additions & 3 deletions src/Bundler.js
Expand Up @@ -207,9 +207,9 @@ class Bundler extends EventEmitter {
});
});
}
this.emit('buildStart', this.entryFiles)

this.emit('buildStart', this.entryFiles);

let isInitialBundle = !this.entryAssets;
let startTime = Date.now();
this.pending = true;
Expand Down
72 changes: 70 additions & 2 deletions src/assets/HTMLAsset.js
Expand Up @@ -62,6 +62,11 @@ const META = {
]
};

const SCRIPT_TYPES = {
'application/javascript': 'js',
'application/json': 'json'
Copy link
Member

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.

Copy link
Member Author

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

};

// Options to be passed to `addURLDependency` for certain tags + attributes
const OPTIONS = {
a: {
Expand Down Expand Up @@ -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({
Copy link
Contributor

Choose a reason for hiding this comment

The 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'
Copy link
Member

Choose a reason for hiding this comment

The 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

Copy link
Member Author

Choose a reason for hiding this comment

The 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(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line might cause issues, related to this issue: #1389
There is currently no way to tell in generated you don't want the hmr code for css assets.

Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

The 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.join('').trim()) {
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)
}
];
}
}

Expand Down
18 changes: 14 additions & 4 deletions src/assets/SASSAsset.js
Expand Up @@ -27,18 +27,28 @@ class SASSAsset extends Asset {
path.dirname(this.name)
);
opts.data = opts.data ? opts.data + os.EOL + code : code;
let type = this.options.rendition ? this.options.rendition.type : path.extname(this.name).toLowerCase().replace('.','');
opts.indentedSyntax = typeof opts.indentedSyntax === 'boolean' ? opts.indentedSyntax : type === 'sass';
let type = this.options.rendition
? this.options.rendition.type
: path
.extname(this.name)
.toLowerCase()
.replace('.', '');
opts.indentedSyntax =
typeof opts.indentedSyntax === 'boolean'
? opts.indentedSyntax
: type === 'sass';

opts.functions = Object.assign({}, opts.functions, {
url: node => {
let filename = this.addURLDependency(node.getValue());
return new sass.types.String(`url(${JSON.stringify(filename)})`);
}
});

opts.importer = opts.importer || [];
opts.importer = Array.isArray(opts.importer) ? opts.importer : [opts.importer];
opts.importer = Array.isArray(opts.importer)
? opts.importer
: [opts.importer];
opts.importer.push((url, prev, done) => {
if (!/^(~|\.\/|\/)/.test(url)) {
url = './' + url;
Expand Down
13 changes: 10 additions & 3 deletions src/transforms/htmlnano.js
Expand Up @@ -4,10 +4,17 @@ const htmlnano = require('htmlnano');
module.exports = async function(asset) {
await asset.parseIfNeeded();

let htmlNanoConfig = await asset.getConfig(
['.htmlnanorc', '.htmlnanorc.js'],
{packageKey: 'htmlnano'}
let htmlNanoConfig = Object.assign(
{},
await asset.getConfig(['.htmlnanorc', '.htmlnanorc.js'], {
packageKey: 'htmlnano'
}),
{
minifyCss: false,
minifyJs: false
}
);

let res = await posthtml([htmlnano(htmlNanoConfig)]).process(asset.ast, {
skipParse: true
});
Expand Down
38 changes: 33 additions & 5 deletions test/html.js
Expand Up @@ -277,11 +277,6 @@ describe('html', function() {
)
);

// minifyJson
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why was this removed?

Copy link
Member Author

Choose a reason for hiding this comment

The 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(
Expand Down Expand Up @@ -576,4 +571,37 @@ 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: ['bg.jpg'],
childBundles: []
},
{
type: 'jpg',
assets: ['img.jpg'],
childBundles: []
}
]
});
});
});
23 changes: 23 additions & 0 deletions test/integration/html-inline-js/index.html
@@ -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>
1 change: 1 addition & 0 deletions test/integration/html-inline-styles/bg.jpg
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions test/integration/html-inline-styles/img.jpg
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
20 changes: 20 additions & 0 deletions test/integration/html-inline-styles/index.html
@@ -0,0 +1,20 @@
<!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>
<p style="background-image: url(./bg.jpg);"></p>
</div>
</body>
</html>