Skip to content

Base scoping hashes on CSS content rather than entire file #1105

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Base scoping hashes on CSS content rather than entire file #1105

Show file tree
Hide file tree
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
10 changes: 8 additions & 2 deletions src/parse/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,14 +185,20 @@ export class Parser {
}
}

function getHashSource (parser: Parser, options: ParserOptions) {
if (options.css === false || !parser.css) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think options.css being false is a reason to use parser.template. If options.css is false, that just means the user doesn't want to have code to inject the <style>, and they will make sure they get the styles another way.

If parser.css is null/undefined, I'm also not sure why we'd need to parse parser.template. In that case, doesn't Svelte just not add the extra attributes to any elements, and thus it doesn't need to have a hash? It seems like it might be possible to not have this getHashSource function and to below simply replace hash: hash(parser.template) with hash: parser.css && hash(parser.css.content.styles).

Copy link
Contributor Author

@emilos emilos Jan 13, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

hey, thanks for the quick reply :)

I had a similar thought at first but I wasn't sure if hash value being set as null | undefined would be an expected value for a hash. Another reason for using the options.css flag is due to the test/runtime/samples/css-false spec, but I might've understood its intent in a wrong way.

After looking at it second time I'm not sure where the logic for the css-false spec sits, I'll try to investigate.

Copy link
Contributor Author

@emilos emilos Jan 13, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@Conduitry just fyi, I've investigated further and I've run into a weird scenario :) if I focus on given spec and change the code to:

hash: parser.css && hash(parser.css.content.styles)

it fails when all specs are run, but it passes correctly when the spec is focused with solo: true (it's not random and happens in 100% cases). Any ideas what could be happening? I guess it is happening since the tests are not isolated but I wasn't able to narrow down the issue yet

return parser.template;
}
return parser.css.content.styles;
}

export default function parse(
template: string,
options: ParserOptions = {}
): Parsed {
const parser = new Parser(template, options);

return {
hash: hash(parser.template),
hash: hash(getHashSource(parser, options)),
html: parser.html,
css: parser.css,
js: parser.js,
Expand Down
8 changes: 4 additions & 4 deletions test/js/samples/collapses-text-around-comments/expected-bundle.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,13 +198,13 @@ function data() {
}

function encapsulateStyles(node) {
setAttribute(node, "svelte-3590263702", "");
setAttribute(node, "svelte-2794052100", "");
}

function add_css() {
var style = createElement("style");
style.id = 'svelte-3590263702-style';
style.textContent = "p[svelte-3590263702],[svelte-3590263702] p{color:red}";
style.id = 'svelte-2794052100-style';
style.textContent = "p[svelte-2794052100],[svelte-2794052100] p{color:red}";
appendNode(style, document.head);
}

Expand Down Expand Up @@ -245,7 +245,7 @@ function SvelteComponent(options) {
init(this, options);
this._state = assign(data(), options.data);

if (!document.getElementById("svelte-3590263702-style")) add_css();
if (!document.getElementById("svelte-2794052100-style")) add_css();

this._fragment = create_main_fragment(this._state, this);

Expand Down
10 changes: 5 additions & 5 deletions test/js/samples/collapses-text-around-comments/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ function data() {
};

function encapsulateStyles(node) {
setAttribute(node, "svelte-3590263702", "");
setAttribute(node, "svelte-2794052100", "");
}

function add_css() {
var style = createElement("style");
style.id = 'svelte-3590263702-style';
style.textContent = "p[svelte-3590263702],[svelte-3590263702] p{color:red}";
style.id = 'svelte-2794052100-style';
style.textContent = "p[svelte-2794052100],[svelte-2794052100] p{color:red}";
appendNode(style, document.head);
}

Expand Down Expand Up @@ -53,7 +53,7 @@ function SvelteComponent(options) {
init(this, options);
this._state = assign(data(), options.data);

if (!document.getElementById("svelte-3590263702-style")) add_css();
if (!document.getElementById("svelte-2794052100-style")) add_css();

this._fragment = create_main_fragment(this._state, this);

Expand All @@ -64,4 +64,4 @@ function SvelteComponent(options) {
}

assign(SvelteComponent.prototype, proto);
export default SvelteComponent;
export default SvelteComponent;
8 changes: 4 additions & 4 deletions test/js/samples/css-media-query/expected-bundle.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,13 +190,13 @@ var proto = {

/* generated by Svelte vX.Y.Z */
function encapsulateStyles(node) {
setAttribute(node, "svelte-2363328337", "");
setAttribute(node, "svelte-3905933315", "");
}

function add_css() {
var style = createElement("style");
style.id = 'svelte-2363328337-style';
style.textContent = "@media(min-width: 1px){div[svelte-2363328337],[svelte-2363328337] div{color:red}}";
style.id = 'svelte-3905933315-style';
style.textContent = "@media(min-width: 1px){div[svelte-3905933315],[svelte-3905933315] div{color:red}}";
appendNode(style, document.head);
}

Expand Down Expand Up @@ -231,7 +231,7 @@ function SvelteComponent(options) {
init(this, options);
this._state = assign({}, options.data);

if (!document.getElementById("svelte-2363328337-style")) add_css();
if (!document.getElementById("svelte-3905933315-style")) add_css();

this._fragment = create_main_fragment(this._state, this);

Expand Down
10 changes: 5 additions & 5 deletions test/js/samples/css-media-query/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
import { appendNode, assign, createElement, detachNode, init, insertNode, noop, proto, setAttribute } from "svelte/shared.js";

function encapsulateStyles(node) {
setAttribute(node, "svelte-2363328337", "");
setAttribute(node, "svelte-3905933315", "");
}

function add_css() {
var style = createElement("style");
style.id = 'svelte-2363328337-style';
style.textContent = "@media(min-width: 1px){div[svelte-2363328337],[svelte-2363328337] div{color:red}}";
style.id = 'svelte-3905933315-style';
style.textContent = "@media(min-width: 1px){div[svelte-3905933315],[svelte-3905933315] div{color:red}}";
appendNode(style, document.head);
}

Expand Down Expand Up @@ -43,7 +43,7 @@ function SvelteComponent(options) {
init(this, options);
this._state = assign({}, options.data);

if (!document.getElementById("svelte-2363328337-style")) add_css();
if (!document.getElementById("svelte-3905933315-style")) add_css();

this._fragment = create_main_fragment(this._state, this);

Expand All @@ -54,4 +54,4 @@ function SvelteComponent(options) {
}

assign(SvelteComponent.prototype, proto);
export default SvelteComponent;
export default SvelteComponent;
4 changes: 2 additions & 2 deletions test/server-side-rendering/samples/styles-nested/_actual.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
div[svelte-1408461649],[svelte-1408461649] div{color:red}
div[svelte-1580141456],[svelte-1580141456] div{color:green}
div[svelte-724714405],[svelte-724714405] div{color:red}
div[svelte-300476157],[svelte-300476157] div{color:green}
6 changes: 3 additions & 3 deletions test/server-side-rendering/samples/styles-nested/_actual.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<div svelte-1408461649>red</div>
<div svelte-1580141456>green: foo</div>
<div svelte-724714405>red</div>
<div svelte-300476157>green: foo</div>



<div svelte-1580141456>green: bar</div>
<div svelte-300476157>green: bar</div>


4 changes: 2 additions & 2 deletions test/server-side-rendering/samples/styles-nested/_expected.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
div[svelte-1408461649],[svelte-1408461649] div{color:red}
div[svelte-1580141456],[svelte-1580141456] div{color:green}
div[svelte-724714405],[svelte-724714405] div{color:red}
div[svelte-300476157],[svelte-300476157] div{color:green}
6 changes: 3 additions & 3 deletions test/server-side-rendering/samples/styles-nested/_expected.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<div svelte-1408461649>red</div>
<div svelte-1580141456>green: foo</div>
<div svelte-724714405>red</div>
<div svelte-300476157>green: foo</div>



<div svelte-1580141456>green: bar</div>
<div svelte-300476157>green: bar</div>


2 changes: 1 addition & 1 deletion test/server-side-rendering/samples/styles/_actual.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
div[svelte-2278551596],[svelte-2278551596] div{color:red}
div[svelte-724714405],[svelte-724714405] div{color:red}
2 changes: 1 addition & 1 deletion test/server-side-rendering/samples/styles/_actual.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<div svelte-2278551596>red</div>
<div svelte-724714405>red</div>
2 changes: 1 addition & 1 deletion test/server-side-rendering/samples/styles/_expected.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
div[svelte-2278551596],[svelte-2278551596] div{color:red}
div[svelte-724714405],[svelte-724714405] div{color:red}
2 changes: 1 addition & 1 deletion test/server-side-rendering/samples/styles/_expected.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<div svelte-2278551596>red</div>
<div svelte-724714405>red</div>
2 changes: 1 addition & 1 deletion test/sourcemaps/samples/css-cascade-false/output.css
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion test/sourcemaps/samples/css/output.css
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.