Skip to content
This repository has been archived by the owner on Sep 14, 2021. It is now read-only.

split and rejoin styles in HtmlSplitter and HtmlRejoiner #40

Closed
wants to merge 1 commit into from
+36 −2
Closed

split and rejoin styles in HtmlSplitter and HtmlRejoiner #40

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 36 additions & 2 deletions src/polymer-project.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,8 +311,8 @@ class HtmlSplitter extends Transform {
for (let i = 0; i < scriptTags.length; i++) {
let scriptTag = scriptTags[i];
let source = dom5.getTextContent(scriptTag);
let typeAtribute = dom5.getAttribute(scriptTag, 'type');
let extension = typeAtribute && extensionsForType[typeAtribute] || 'js';
let typeAttribute = dom5.getAttribute(scriptTag, 'type');
Copy link
Contributor

Choose a reason for hiding this comment

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

haha, nice catch

let extension = typeAttribute && extensionsForType[typeAttribute] || 'js';
let childFilename = `${osPath.basename(filePath)}_script_${i}.${extension}`;
let childPath = osPath.join(osPath.dirname(filePath), childFilename);
scriptTag.childNodes = [];
Expand All @@ -327,6 +327,24 @@ class HtmlSplitter extends Transform {
this.push(scriptFile);
}

for (let i = 0; i < styleTags.length; i++) {
let styleTag = styleTags[i];
let source = dom5.getTextContent(styleTag);
let extension = 'css';
let childFilename = `${osPath.basename(filePath)}_style_${i}.${extension}`;
let childPath = osPath.join(osPath.dirname(filePath), childFilename);
styleTag.childNodes = [];
dom5.setAttribute(styleTag, 'src', childFilename);
Copy link
Contributor

Choose a reason for hiding this comment

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

This will make a tag like: <style src="x.css"></style>, when it needs to be <link rel="stylesheet" href="x.css">

Copy link

Choose a reason for hiding this comment

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

A <link> tag is definitely what should be here, but there's no support for the include property on an external stylesheet. Many styles will still be broken when split.

Copy link
Contributor

Choose a reason for hiding this comment

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

but <style src=> isn't valid

Copy link

Choose a reason for hiding this comment

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

I'm not saying <style src=> isn't wrong. Just pointing out that <link include=> also isn't valid. If split without rejoin on a polymer project is to be supported, more needs to be done here to split these styles. Any ideas for how to handle that?

Copy link
Contributor

Choose a reason for hiding this comment

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

True <link include=> isn't valid, and so we shouldn't generate that :)

If a style tag has a include attribute we have to be more careful about what we generate. Possibly a <style include=><style> and a <link rel=stylesheet>.

@sorvell is there any valid transformation that externalizes a <script include=> tag?

let styleFile = new File({
cwd: file.cwd,
base: file.base,
path: childPath,
contents: new Buffer(source),
});
this._project.addSplitPath(filePath, childPath);
this.push(styleFile);
}

let splitContents = dom5.serialize(doc);
let newFile = new File({
cwd: file.cwd,
Expand Down Expand Up @@ -356,6 +374,11 @@ class HtmlRejoiner extends Transform {
pred.hasAttr('src')
);

static isExternalStyle = pred.AND(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this used anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be used on line 425 but is missing. Will change it to match a <link> tag as well for matching the changes below.

pred.hasTagName('style'),
pred.hasAttr('src')
);

_project: PolymerProject;

constructor(project: PolymerProject) {
Expand Down Expand Up @@ -412,6 +435,17 @@ class HtmlRejoiner extends Transform {
}
}

for (let i = 0; i < styleTags.length; i++) {
let styleTag = styleTags[i];
let srcAttribute = dom5.getAttribute(styleTag, 'src');
let stylePath = osPath.join(osPath.dirname(splitFile.path), srcAttribute);
if (splitFile.parts.has(stylePath)) {
let styleSource = splitFile.parts.get(stylePath);
dom5.setTextContent(styleTag, styleSource);
dom5.removeAttribute(styleTag, 'src');
}
}

let joinedContents = dom5.serialize(doc);

return new File({
Expand Down