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

Transitions #525

Merged
merged 34 commits into from
May 3, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
2784ae0
parse transition directives
Rich-Harris Apr 25, 2017
4fa7765
failing test for intro transition
Rich-Harris Apr 25, 2017
9df2243
Merge branch 'master' into gh-7
Rich-Harris Apr 25, 2017
d0c0fbc
add transitions property to default export, track intros/outros in Block
Rich-Harris Apr 26, 2017
8ccad1f
first working intro transition, woooo
Rich-Harris Apr 26, 2017
6ed2a6c
update tests
Rich-Harris Apr 26, 2017
53c5c32
allow parameter-less transitions
Rich-Harris Apr 26, 2017
2a5b0ee
support very basic outro transitions
Rich-Harris Apr 26, 2017
7f76ab2
Merge branch 'master' into gh-7
Rich-Harris Apr 30, 2017
aa67f8b
abort transitions
Rich-Harris Apr 30, 2017
45a9ce0
handle bidirectional transitions differently
Rich-Harris Apr 30, 2017
806b098
CSS transitions
Rich-Harris Apr 30, 2017
d63f80f
never abort transitions, they are either bidi or non-abortable
Rich-Harris Apr 30, 2017
5638a76
restart animations on secondary intro, various bits of cleanup
Rich-Harris Apr 30, 2017
5bee31f
get basic intro transition test passing
Rich-Harris May 1, 2017
26ed672
some more transition tests, albeit somewhat ugly
Rich-Harris May 1, 2017
dfe00d8
support dynamic simple if-blocks
Rich-Harris May 1, 2017
ec0e4a6
support transitions in compound if-blocks
Rich-Harris May 1, 2017
07f6ec5
only apply easing function once!
Rich-Harris May 1, 2017
f5bc3e3
remove method is unused
Rich-Harris May 1, 2017
f76fac2
tighten up transition tests
Rich-Harris May 1, 2017
65064cb
improve deindent slightly — allow inline false expressions (which get…
Rich-Harris May 1, 2017
a2cd983
intro transitions in each-blocks
Rich-Harris May 1, 2017
2d533f9
remove redundant ternary
Rich-Harris May 1, 2017
42af2bb
fix mount order of keyed each-block with intros
Rich-Harris May 1, 2017
f06eced
unkeyed each-blocks with outros
Rich-Harris May 1, 2017
22ac50a
outros on keyed each-blocks
Rich-Harris May 1, 2017
b8affd4
simplify/unify transitions
Rich-Harris May 1, 2017
8da7019
rename styles method to css - less ambiguity over what it returns, no…
Rich-Harris May 3, 2017
dee8694
merge master -> gh-7
Rich-Harris May 3, 2017
e796fef
stringify helpers before bundling
Rich-Harris May 3, 2017
bdfa01b
fix script path, duh
Rich-Harris May 3, 2017
d7b3f2e
lint after build
Rich-Harris May 3, 2017
90adb3b
gah node 4
Rich-Harris May 3, 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
7 changes: 5 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@
"precodecov": "npm run coverage",
"lint": "eslint src test/*.js",
"build": "npm run build:main && npm run build:shared && npm run build:ssr",
"build:main": "rollup -c rollup/rollup.config.main.js",
"build:main": "node src/shared/_build.js && rollup -c rollup/rollup.config.main.js",
"build:shared": "rollup -c rollup/rollup.config.shared.js",
"build:ssr": "rollup -c rollup/rollup.config.ssr.js",
"dev": "node src/shared/_build.js && rollup -c rollup/rollup.config.main.js -w",
"dev:shared": "rollup -c rollup/rollup.config.shared.js -w",
"pretest": "npm run build",
"prepublish": "npm run lint && npm run build"
"prepublish": "npm run build && npm run lint"
},
"repository": {
"type": "git",
Expand Down Expand Up @@ -72,6 +74,7 @@
"rollup-plugin-commonjs": "^7.0.0",
"rollup-plugin-json": "^2.1.0",
"rollup-plugin-node-resolve": "^2.0.0",
"rollup-watch": "^3.2.2",
"source-map": "^0.5.6",
"source-map-support": "^0.4.8"
},
Expand Down
3 changes: 2 additions & 1 deletion src/generators/Generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export default class Generator {
this.helpers = new Set();
this.components = new Set();
this.events = new Set();
this.transitions = new Set();
this.importedComponents = new Map();

this.bindingGroups = [];
Expand Down Expand Up @@ -328,7 +329,7 @@ export default class Generator {
});
}

[ 'helpers', 'events', 'components' ].forEach( key => {
[ 'helpers', 'events', 'components', 'transitions' ].forEach( key => {
if ( templateProperties[ key ] ) {
templateProperties[ key ].value.properties.forEach( prop => {
this[ key ].add( prop.key.name );
Expand Down
64 changes: 64 additions & 0 deletions src/generators/dom/Block.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,17 @@ export default class Block {
create: new CodeBuilder(),
mount: new CodeBuilder(),
update: new CodeBuilder(),
intro: new CodeBuilder(),
outro: new CodeBuilder(),
detach: new CodeBuilder(),
detachRaw: new CodeBuilder(),
destroy: new CodeBuilder()
};

this.hasIntroMethod = false; // a block could have an intro method but not intro transitions, e.g. if a sibling block has intros
this.hasOutroMethod = false;
this.outros = 0;

this.aliases = new Map();
this.variables = new Map();
this.getUniqueName = this.generator.getUniqueNameMaker( options.params );
Expand Down Expand Up @@ -100,6 +106,20 @@ export default class Block {
}

render () {
let introing;
const hasIntros = !this.builders.intro.isEmpty();
if ( hasIntros ) {
introing = this.getUniqueName( 'introing' );
this.addVariable( introing );
}

let outroing;
const hasOutros = !this.builders.outro.isEmpty();
if ( hasOutros ) {
outroing = this.getUniqueName( 'outroing' );
this.addVariable( outroing );
}

if ( this.variables.size ) {
const variables = Array.from( this.variables.keys() )
.map( key => {
Expand Down Expand Up @@ -157,6 +177,50 @@ export default class Block {
}
}

if ( this.hasIntroMethod ) {
if ( hasIntros ) {
properties.addBlock( deindent`
intro: function ( ${this.target}, anchor ) {
if ( ${introing} ) return;
${introing} = true;
${hasOutros && `${outroing} = false;`}

${this.builders.intro}

this.mount( ${this.target}, anchor );
},
` );
} else {
properties.addBlock( deindent`
intro: function ( ${this.target}, anchor ) {
this.mount( ${this.target}, anchor );
},
` );
}
}

if ( this.hasOutroMethod ) {
if ( hasOutros ) {
properties.addBlock( deindent`
outro: function ( ${this.alias( 'outrocallback' )} ) {
if ( ${outroing} ) return;
${outroing} = true;
${hasIntros && `${introing} = false;`}

var ${this.alias( 'outros' )} = ${this.outros};

${this.builders.outro}
},
` );
} else {
properties.addBlock( deindent`
outro: function ( outrocallback ) {
outrocallback();
},
` );
}
}

if ( this.builders.destroy.isEmpty() ) {
properties.addBlock( `destroy: ${this.generator.helper( 'noop' )}` );
} else {
Expand Down
40 changes: 24 additions & 16 deletions src/generators/dom/index.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import MagicString from 'magic-string';
import { parse } from 'acorn';
import { parseExpressionAt } from 'acorn';
import annotateWithScopes from '../../utils/annotateWithScopes.js';
import isReference from '../../utils/isReference.js';
import { walk } from 'estree-walker';
import deindent from '../../utils/deindent.js';
import CodeBuilder from '../../utils/CodeBuilder.js';
import visit from './visit.js';
import { nameMap, sharedMap } from './sharedNames.js';
import shared from './shared.js';
import Generator from '../Generator.js';
import preprocess from './preprocess.js';

Expand All @@ -25,7 +25,7 @@ class DomGenerator extends Generator {
}

helper ( name ) {
if ( this.options.dev && sharedMap.has( `${name}Dev` ) ) {
if ( this.options.dev && `${name}Dev` in shared ) {
name = `${name}Dev`;
}

Expand Down Expand Up @@ -138,7 +138,7 @@ export default function dom ( parsed, source, options ) {
builders.init.addLine( `if ( !${generator.alias( 'added_css' )} ) ${generator.alias( 'add_css' )}();` );
}

if ( generator.hasComponents ) {
if ( generator.hasComponents || generator.hasIntroTransitions ) {
builders.init.addLine( `this._renderHooks = [];` );
}

Expand All @@ -158,7 +158,7 @@ export default function dom ( parsed, source, options ) {
` );
}

if ( generator.hasComponents ) {
if ( generator.hasComponents || generator.hasIntroTransitions ) {
const statement = `this._flush();`;

builders.init.addBlock( statement );
Expand All @@ -168,7 +168,7 @@ export default function dom ( parsed, source, options ) {
if ( templateProperties.oncreate ) {
builders.init.addBlock( deindent`
if ( options._root ) {
options._root._renderHooks.push({ fn: ${generator.alias( 'template' )}.oncreate, context: this });
options._root._renderHooks.push( ${generator.alias( 'template' )}.oncreate.bind( this ) );
Copy link
Member

Choose a reason for hiding this comment

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

Since .bind is slower than pretty much every other option of calling a function later, we could do something like

var self = this;
options._root._renderHooks.push( function () {
  ${ generator.alias( ' template' ) }.oncreate.call ( self );
} );

which would be slightly faster. Here's a test showing this: https://jsperf.com/bind-vs-self-closure (interestingly enough the calling of a .binded function is no slower than the closure in firefox, but in chrome it's 10x slower)

Copy link
Member Author

Choose a reason for hiding this comment

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

That's probably better, yeah. If we wanted to be really clever, the fastest way to run oncreate would probably be to rewrite references to this, the same way we do in non-hoisted event handlers:

// this...
oncreate () {
  this.observe( 'foo', function ( foo ) {
    this.refs.whatever.value = foo.toUpperCase();
  });
}

// ...becomes this:
oncreate ( component ) {
  component.observe( 'foo', function ( foo ) {
    this.refs.whatever.value = foo.toUpperCase();
  });
}

Or is that a terrible idea?

Copy link
Member Author

Choose a reason for hiding this comment

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

(obviously that only works if oncreate is declared inline — if it's a reference we'd still need to .call it)

} else {
${generator.alias( 'template' )}.oncreate.call( this );
}
Expand Down Expand Up @@ -218,7 +218,7 @@ export default function dom ( parsed, source, options ) {

this._handlers = Object.create( null );

this._root = options._root;
this._root = options._root || this;
this._yield = options._yield;

${builders.init}
Expand Down Expand Up @@ -275,20 +275,20 @@ export default function dom ( parsed, source, options ) {
);
} else {
generator.uses.forEach( key => {
const str = sharedMap.get( key );
const str = shared[ key ];
const code = new MagicString( str );
const fn = parse( str ).body[0];
const expression = parseExpressionAt( str, 0 );

let scope = annotateWithScopes( fn );
let scope = annotateWithScopes( expression );

walk( fn, {
walk( expression, {
enter ( node, parent ) {
if ( node._scope ) scope = node._scope;

if ( node.type === 'Identifier' && isReference( node, parent ) && !scope.has( node.name ) ) {
if ( nameMap.has( node.name ) ) {
if ( node.name in shared ) {
// this helper function depends on another one
const dependency = nameMap.get( node.name );
const dependency = node.name;
generator.uses.add( dependency );

const alias = generator.alias( dependency );
Expand All @@ -302,10 +302,18 @@ export default function dom ( parsed, source, options ) {
}
});

const alias = generator.alias( key );
if ( alias !== fn.id.name ) code.overwrite( fn.id.start, fn.id.end, alias );
if ( key === 'transitionManager' ) { // special case
const global = `_svelteTransitionManager`;

builders.main.addBlock( code.toString() );
builders.main.addBlock(
`var ${generator.alias( 'transitionManager' )} = window.${global} || ( window.${global} = ${code});`
);
} else {
const alias = generator.alias( expression.id.name );
if ( alias !== expression.id.name ) code.overwrite( expression.id.start, expression.id.end, alias );

builders.main.addBlock( code.toString() );
}
});
}

Expand Down
15 changes: 15 additions & 0 deletions src/generators/dom/preprocess.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ const preprocessors = {
IfBlock: ( generator, block, state, node ) => {
const blocks = [];
let dynamic = false;
let hasIntros = false;
let hasOutros = false;

function attachBlocks ( node ) {
const dependencies = block.findDependencies( node.expression );
Expand All @@ -78,6 +80,9 @@ const preprocessors = {
block.addDependencies( node._block.dependencies );
}

if ( node._block.hasIntroMethod ) hasIntros = true;
if ( node._block.hasOutroMethod ) hasOutros = true;

if ( isElseIf( node.else ) ) {
attachBlocks( node.else.children[0] );
} else if ( node.else ) {
Expand All @@ -101,6 +106,8 @@ const preprocessors = {

blocks.forEach( block => {
block.hasUpdateMethod = dynamic;
block.hasIntroMethod = hasIntros;
block.hasOutroMethod = hasOutros;
});

generator.blocks.push( ...blocks );
Expand Down Expand Up @@ -200,6 +207,14 @@ const preprocessors = {
const dependencies = block.findDependencies( attribute.value );
block.addDependencies( dependencies );
}

else if ( attribute.type === 'Transition' ) {
if ( attribute.intro ) generator.hasIntroTransitions = block.hasIntroMethod = true;
if ( attribute.outro ) {
generator.hasOutroTransitions = block.hasOutroMethod = true;
block.outros += 1;
}
}
});

if ( node.children.length ) {
Expand Down
12 changes: 0 additions & 12 deletions src/generators/dom/sharedNames.js

This file was deleted.

2 changes: 1 addition & 1 deletion src/generators/dom/visitors/Component/Component.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export default function visitComponent ( generator, block, state, node ) {

const componentInitProperties = [
`target: ${!isToplevel ? state.parentNode: 'null'}`,
`_root: ${block.component}._root || ${block.component}`
`_root: ${block.component}._root`
];

// Component has children, put them in a separate {{yield}} block
Expand Down
Loading