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

Only apply svelte-123xyz attributes where necessary #680

Merged
merged 23 commits into from
Jul 5, 2017
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
0cbd201
set up tests for omitted scoping attributes
Rich-Harris Jun 26, 2017
fae9036
only apply svelte-123xyz attributes where necessary (WIP)
Rich-Harris Jun 26, 2017
72f39fd
Merge branch 'master' into gh-679
Rich-Harris Jun 26, 2017
ab40007
move CSS analysis into preprocess
Rich-Harris Jun 27, 2017
f97ac27
handle :global(...) styles
Rich-Harris Jun 30, 2017
d9aa3ec
more :global(...) handling
Rich-Harris Jul 1, 2017
5499327
refactoring, and more :global(...) fixes
Rich-Harris Jul 1, 2017
f485c25
create reusable walkRules helper
Rich-Harris Jul 1, 2017
b72684e
handle universal selectors
Rich-Harris Jul 1, 2017
d2f5296
handle attribute selectors with = operator
Rich-Harris Jul 1, 2017
74d15ea
handle empty attributes
Rich-Harris Jul 1, 2017
3dfe92b
handle ~= attribute selector operator
Rich-Harris Jul 1, 2017
45dd99d
implement all attribute selector operators
Rich-Harris Jul 1, 2017
7b289e9
implement ID selectors, refactor
Rich-Harris Jul 1, 2017
2ec0a85
apply css optimisations to SSR
Rich-Harris Jul 2, 2017
f79e901
bug fix
Rich-Harris Jul 2, 2017
6f01d5f
refactor a bit, fix typing error
Rich-Harris Jul 3, 2017
205bcfa
warn on unused selectors
Rich-Harris Jul 3, 2017
2c9fb31
simplify tests
Rich-Harris Jul 3, 2017
5f6db88
allow :global(...) selectors to have trailing modifiers
Rich-Harris Jul 3, 2017
1290759
Merge branch 'gh-678' into gh-679
Rich-Harris Jul 3, 2017
0c33eb4
fix tests
Rich-Harris Jul 3, 2017
7a752df
fix handling of modified :global(...) selectors
Rich-Harris Jul 5, 2017
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
19 changes: 19 additions & 0 deletions src/generators/Generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import annotateWithScopes from '../utils/annotateWithScopes';
import clone from '../utils/clone';
import DomBlock from './dom/Block';
import SsrBlock from './server-side-rendering/Block';
import extractSelectors from './extractSelectors';
import { Node, Parsed, CompileOptions } from '../interfaces';

const test = typeof global !== 'undefined' && global.__svelte_test;
Expand Down Expand Up @@ -40,6 +41,8 @@ export default class Generator {
cssId: string;
usesRefs: boolean;

selectors: any[]; // TODO how to indicate it takes `(Node[]) => boolean` functions?
Copy link
Member

@Conduitry Conduitry Jul 2, 2017

Choose a reason for hiding this comment

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

selectors: ((nodes: Node[]) => boolean)[]; I believe

edit: Although this type doesn't seem to match how it's used later. extractSelectors returns simply Node[].

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch. Have refactored it a bit so that it's a bit more TS-friendly (and possibly a bit clearer anyway) — a Selector class with an apply method


importedNames: Set<string>;
aliases: Map<string, string>;
usedNames: Set<string>;
Expand Down Expand Up @@ -76,6 +79,8 @@ export default class Generator {
this.cssId = parsed.css ? `svelte-${parsed.hash}` : '';
this.usesRefs = false;

this.selectors = extractSelectors(parsed.css);

// allow compiler to deconflict user's `import { get } from 'whatever'` and
// Svelte's builtin `import { get, ... } from 'svelte/shared.ts'`;
this.importedNames = new Set();
Expand Down Expand Up @@ -211,6 +216,20 @@ export default class Generator {
};
}

applyCss(node: Node, stack: Node[]) {
if (!this.cssId) return;

if (this.cascade) {
if (stack.length === 0) node._needsCssAttribute = true;
return;
}

for (let i = 0; i < this.selectors.length; i += 1) {
const selector = this.selectors[i];
selector.apply(node, stack);
}
}

findDependencies(
contextDependencies: Map<string, string[]>,
indexes: Map<string, string>,
Expand Down
2 changes: 1 addition & 1 deletion src/generators/dom/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export default function dom(
const { block, state } = preprocess(generator, namespace, parsed.html);

parsed.html.children.forEach((node: Node) => {
visit(generator, block, state, node);
visit(generator, block, state, node, []);
});

const builder = new CodeBuilder();
Expand Down
31 changes: 24 additions & 7 deletions src/generators/dom/preprocess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const preprocessors = {
block: Block,
state: State,
node: Node,
elementStack: Node[],
stripWhitespace: boolean
) => {
const dependencies = block.findDependencies(node.expression);
Expand All @@ -55,6 +56,7 @@ const preprocessors = {
block: Block,
state: State,
node: Node,
elementStack: Node[],
stripWhitespace: boolean
) => {
const dependencies = block.findDependencies(node.expression);
Expand All @@ -66,7 +68,14 @@ const preprocessors = {
node._state = getChildState(state, { basename, name });
},

Text: (generator: DomGenerator, block: Block, state: State, node: Node, stripWhitespace: boolean) => {
Text: (
generator: DomGenerator,
block: Block,
state: State,
node: Node,
elementStack: Node[],
stripWhitespace: boolean
) => {
node._state = getChildState(state);

if (!/\S/.test(node.data)) {
Expand All @@ -83,6 +92,7 @@ const preprocessors = {
block: Block,
state: State,
node: Node,
elementStack: Node[],
stripWhitespace: boolean,
nextSibling: Node
) => {
Expand All @@ -102,7 +112,7 @@ const preprocessors = {
node._state = getChildState(state);

blocks.push(node._block);
preprocessChildren(generator, node._block, node._state, node, stripWhitespace, node);
preprocessChildren(generator, node._block, node._state, node, elementStack, stripWhitespace, nextSibling);

if (node._block.dependencies.size > 0) {
dynamic = true;
Expand All @@ -127,6 +137,7 @@ const preprocessors = {
node.else._block,
node.else._state,
node.else,
elementStack,
stripWhitespace,
nextSibling
);
Expand Down Expand Up @@ -154,6 +165,7 @@ const preprocessors = {
block: Block,
state: State,
node: Node,
elementStack: Node[],
stripWhitespace: boolean,
nextSibling: Node
) => {
Expand Down Expand Up @@ -202,7 +214,7 @@ const preprocessors = {
});

generator.blocks.push(node._block);
preprocessChildren(generator, node._block, node._state, node, stripWhitespace, nextSibling);
preprocessChildren(generator, node._block, node._state, node, elementStack, stripWhitespace, nextSibling);
block.addDependencies(node._block.dependencies);
node._block.hasUpdateMethod = node._block.dependencies.size > 0;

Expand All @@ -219,6 +231,7 @@ const preprocessors = {
node.else._block,
node.else._state,
node.else,
elementStack,
stripWhitespace,
nextSibling
);
Expand All @@ -231,6 +244,7 @@ const preprocessors = {
block: Block,
state: State,
node: Node,
elementStack: Node[],
stripWhitespace: boolean,
nextSibling: Node
) => {
Expand Down Expand Up @@ -315,6 +329,8 @@ const preprocessors = {
: state.namespace,
allUsedContexts: [],
});

generator.applyCss(node, elementStack);
}

if (node.children.length) {
Expand All @@ -328,12 +344,12 @@ const preprocessors = {
});

generator.blocks.push(node._block);
preprocessChildren(generator, node._block, node._state, node, stripWhitespace, nextSibling);
preprocessChildren(generator, node._block, node._state, node, elementStack, stripWhitespace, nextSibling);
block.addDependencies(node._block.dependencies);
node._block.hasUpdateMethod = node._block.dependencies.size > 0;
} else {
if (node.name === 'pre' || node.name === 'textarea') stripWhitespace = false;
preprocessChildren(generator, block, node._state, node, stripWhitespace, nextSibling);
preprocessChildren(generator, block, node._state, node, elementStack.concat(node), stripWhitespace, nextSibling);
}
}
},
Expand All @@ -344,6 +360,7 @@ function preprocessChildren(
block: Block,
state: State,
node: Node,
elementStack: Node[],
stripWhitespace: boolean,
nextSibling: Node
) {
Expand Down Expand Up @@ -373,7 +390,7 @@ function preprocessChildren(

cleaned.forEach((child: Node, i: number) => {
const preprocessor = preprocessors[child.type];
if (preprocessor) preprocessor(generator, block, state, child, stripWhitespace, cleaned[i + 1] || nextSibling);
if (preprocessor) preprocessor(generator, block, state, child, elementStack, stripWhitespace, cleaned[i + 1] || nextSibling);

if (lastChild) {
lastChild.next = child;
Expand Down Expand Up @@ -432,7 +449,7 @@ export default function preprocess(
};

generator.blocks.push(block);
preprocessChildren(generator, block, state, node, true, null);
preprocessChildren(generator, block, state, node, [], true, null);
block.hasUpdateMethod = block.dependencies.size > 0;

return { block, state };
Expand Down
8 changes: 5 additions & 3 deletions src/generators/dom/visit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@ import visitors from './visitors/index';
import { DomGenerator } from './index';
import Block from './Block';
import { Node } from '../../interfaces';
import { State } from './interfaces';

export default function visit(
generator: DomGenerator,
block: Block,
state,
node: Node
state: State,
node: Node,
elementStack: Node[]
) {
const visitor = visitors[node.type];
visitor(generator, block, state, node);
visitor(generator, block, state, node, elementStack);
}
5 changes: 3 additions & 2 deletions src/generators/dom/visitors/Component/Component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ export default function visitComponent(
generator: DomGenerator,
block: Block,
state: State,
node: Node
node: Node,
elementStack: Node[]
) {
const hasChildren = node.children.length > 0;
const name = block.getUniqueName(
Expand Down Expand Up @@ -121,7 +122,7 @@ export default function visitComponent(
const childBlock = node._block;

node.children.forEach((child: Node) => {
visit(generator, childBlock, childState, child);
visit(generator, childBlock, childState, child, elementStack);
});

const yieldFragment = block.getUniqueName(`${name}_yield_fragment`);
Expand Down
7 changes: 4 additions & 3 deletions src/generators/dom/visitors/EachBlock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ export default function visitEachBlock(
generator: DomGenerator,
block: Block,
state: State,
node: Node
node: Node,
elementStack: Node[]
) {
const each_block = generator.getUniqueName(`each_block`);
const create_each_block = node._block.name;
Expand Down Expand Up @@ -117,12 +118,12 @@ export default function visitEachBlock(
}

node.children.forEach((child: Node) => {
visit(generator, node._block, node._state, child);
visit(generator, node._block, node._state, child, elementStack);
});

if (node.else) {
node.else.children.forEach((child: Node) => {
visit(generator, node.else._block, node.else._state, child);
visit(generator, node.else._block, node.else._state, child, elementStack);
});
}
}
Expand Down
10 changes: 6 additions & 4 deletions src/generators/dom/visitors/Element/Element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,15 @@ export default function visitElement(
generator: DomGenerator,
block: Block,
state: State,
node: Node
node: Node,
elementStack: Node[]
) {
if (node.name in meta) {
return meta[node.name](generator, block, node);
}

if (generator.components.has(node.name) || node.name === ':Self') {
return visitComponent(generator, block, state, node);
return visitComponent(generator, block, state, node, elementStack);
}

const childState = node._state;
Expand Down Expand Up @@ -80,7 +81,8 @@ export default function visitElement(
}

// add CSS encapsulation attribute
if (generator.cssId && (!generator.cascade || state.isTopLevel)) {
// TODO add a helper for this, rather than repeating it
if (node._needsCssAttribute) {
block.builders.hydrate.addLine(
`@setAttribute( ${name}, '${generator.cssId}', '' );`
);
Expand Down Expand Up @@ -181,7 +183,7 @@ export default function visitElement(
}

node.children.forEach((child: Node) => {
visit(generator, block, childState, child);
visit(generator, block, childState, child, elementStack.concat(node));
});

if (node.lateUpdate) {
Expand Down
19 changes: 11 additions & 8 deletions src/generators/dom/visitors/IfBlock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ function getBranches(
generator: DomGenerator,
block: Block,
state: State,
node: Node
node: Node,
elementStack: Node[]
) {
const branches = [
{
Expand All @@ -31,11 +32,11 @@ function getBranches(
},
];

visitChildren(generator, block, state, node);
visitChildren(generator, block, state, node, elementStack);

if (isElseIf(node.else)) {
branches.push(
...getBranches(generator, block, state, node.else.children[0])
...getBranches(generator, block, state, node.else.children[0], elementStack)
);
} else {
branches.push({
Expand All @@ -47,7 +48,7 @@ function getBranches(
});

if (node.else) {
visitChildren(generator, block, state, node.else);
visitChildren(generator, block, state, node.else, elementStack);
}
}

Expand All @@ -58,26 +59,28 @@ function visitChildren(
generator: DomGenerator,
block: Block,
state: State,
node: Node
node: Node,
elementStack: Node[]
) {
node.children.forEach((child: Node) => {
visit(generator, node._block, node._state, child);
visit(generator, node._block, node._state, child, elementStack);
});
}

export default function visitIfBlock(
generator: DomGenerator,
block: Block,
state: State,
node: Node
node: Node,
elementStack: Node[]
) {
const name = generator.getUniqueName(`if_block`);
const anchor = node.needsAnchor
? block.getUniqueName(`${name}_anchor`)
: (node.next && node.next._state.name) || 'null';
const params = block.params.join(', ');

const branches = getBranches(generator, block, state, node);
const branches = getBranches(generator, block, state, node, elementStack);

const hasElse = isElseBranch(branches[branches.length - 1]);
const if_name = hasElse ? '' : `if ( ${name} ) `;
Expand Down
Loading