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

attach globals to state object on initialisation #924

Merged
merged 18 commits into from
Nov 23, 2017
Merged

attach globals to state object on initialisation #924

merged 18 commits into from
Nov 23, 2017

Conversation

Rich-Harris
Copy link
Member

@Rich-Harris Rich-Harris commented Nov 14, 2017

Ref #908. Just leaving this failing test here for now. Not totally sure what the best approach to fixing it is.

@Rich-Harris
Copy link
Member Author

Idea: any globals that are used in the template, we add them to the state object when the component is initialised (if they don't already exist). That way, we dispense with the need to do things like this on update:

var each_value = ('Object' in state ? state.Object : Object).keys(state.todos);

It means adding additional those properties to state, but I don't see that as a problem.

@Rich-Harris
Copy link
Member Author

Turns out that approach makes a lot of sense. Unfortunately, it doesn't currently work for SSR, because generator.expectedProperties isn't populated, because that happens in generator.findDependencies, which... is only called by the DOM compiler.

Rather than hacking in a fix, I reckon it's probably time to deal with that design flaw — resolving all those dependencies is probably something that needs to happen in generator.parseJs. It'll mean a bit of refactoring.

@Rich-Harris Rich-Harris changed the title [WIP] failing test for #908 [WIP] attach globals to state object on initialisation Nov 22, 2017
@Rich-Harris
Copy link
Member Author

Feeling pretty good about this. Generator now has a walkTemplate method which is invoked upon construction — all the dependency tracking happens at that point, rather than later in generator.contextualise. This means the DOM and SSR compilers are both going through the same process, and everything happens in an order that's somewhat easier to reason about.

Phase 2 of this refactor will probably be to move all the stuff that's currently happening in generator.contextualise into walkTemplate — i.e. the code transformation itself.

@Rich-Harris Rich-Harris changed the title [WIP] attach globals to state object on initialisation attach globals to state object on initialisation Nov 23, 2017
@Rich-Harris Rich-Harris merged commit 5190144 into master Nov 23, 2017
@Rich-Harris Rich-Harris deleted the gh-908 branch November 23, 2017 03:02
This was referenced Nov 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant