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

feat(platform-server): add an API to transfer state from server #19134

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
10 participants
@vikerman
Copy link
Contributor

commented Sep 11, 2017

TransferState provides a shared store that is transferred from the
server to client. To use it import BrowserTransferStateModule from the
client app module and ServerTransferStateModule from the server app
module.

Design doc: https://docs.google.com/document/d/1Hv3VTRQC0H5yPe8ivJXaGst93yRAtD6nEFkQDLiK_IU/preview

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

@googlebot googlebot added the cla: yes label Sep 11, 2017

@mary-poppins

This comment has been minimized.

Copy link

commented Sep 11, 2017

@mary-poppins

This comment has been minimized.

Copy link

commented Sep 11, 2017

@mary-poppins

This comment has been minimized.

Copy link

commented Sep 11, 2017

@mary-poppins

This comment has been minimized.

Copy link

commented Sep 11, 2017

@mary-poppins

This comment has been minimized.

Copy link

commented Sep 12, 2017

@Toxicable Toxicable referenced this pull request Sep 12, 2017

Closed

tracking: features #794

7 of 12 tasks complete

@m98 m98 referenced this pull request Sep 12, 2017

Closed

feat: NgCache Service? #4389

@mary-poppins

This comment has been minimized.

Copy link

commented Sep 12, 2017

@mary-poppins

This comment has been minimized.

Copy link

commented Sep 12, 2017

@vikerman vikerman requested review from alxhub and mhevery Sep 13, 2017

@mary-poppins

This comment has been minimized.

Copy link

commented Sep 13, 2017

@mary-poppins

This comment has been minimized.

Copy link

commented Sep 13, 2017

You can preview 106310d at https://pr19134-106310d.ngbuilds.io/.

@mary-poppins

This comment has been minimized.

Copy link

commented Sep 13, 2017

*
* @experimental
*/
export interface TransferKey<T> { readonly key: string; }

This comment has been minimized.

Copy link
@alxhub

alxhub Sep 13, 2017

Contributor

StateKey?

This comment has been minimized.

Copy link
@vikerman

vikerman Sep 13, 2017

Author Contributor

Done

*/
@Injectable()
export class TransferStore {
private store: {[k: string]: {}} = {};

This comment has been minimized.

Copy link
@alxhub

alxhub Sep 13, 2017

Contributor

Nit: {[k: string]: {}} implies that there is a value for every possible key.

{[k: string]: {}|undefined}

This comment has been minimized.

Copy link
@vikerman

vikerman Sep 13, 2017

Author Contributor

Done

private onSerializeCallbacks: {[k: string]: () => {}} = {};

/** @internal */
static init(initState: {}) {

This comment has been minimized.

Copy link
@alxhub

alxhub Sep 13, 2017

Contributor

Okay to use constructors for injected classes.

This comment has been minimized.

Copy link
@vikerman

vikerman Sep 13, 2017

Author Contributor

Done

* Get the value corresponding to a key. Return `defaultValue` if key is not found.
*/
get<T>(key: TransferKey<T>, defaultValue: T): T {
return this.store[key.key] as T || defaultValue;

This comment has been minimized.

Copy link
@alxhub

alxhub Sep 13, 2017

Contributor

What happens if T is nullable?

*/
@Injectable()
export class TransferStore {
private store: {[k: string]: {}} = {};

This comment has been minimized.

Copy link
@alxhub

alxhub Sep 13, 2017

Contributor

Is the state guaranteed to be an object {}? Can't it also be a primitive value? Maybe this is a use case for any?

This comment has been minimized.

Copy link
@Toxicable

Toxicable Sep 13, 2017

Contributor

{} can also be a primitive
see https://goo.gl/2SWn85

This comment has been minimized.

Copy link
@vikerman

vikerman Sep 13, 2017

Author Contributor

{} encompasses primitive values also

import {APP_ID, NgModule} from '@angular/core';
import {DOCUMENT, TransferStore, ɵescapeHtml as escapeHtml} from '@angular/platform-browser';

import {} from './server';

This comment has been minimized.

Copy link
@alxhub

alxhub Sep 13, 2017

Contributor

Whoops.

This comment has been minimized.

Copy link
@vikerman

vikerman Sep 13, 2017

Author Contributor

Done

@@ -41,5 +41,9 @@ export declare function renderModuleFactory<T>(moduleFactory: NgModuleFactory<T>
export declare class ServerModule {
}

/** @experimental */
export declare class ServerTransferStoreModule {

This comment has been minimized.

Copy link
@alxhub

alxhub Sep 13, 2017

Contributor

ServerTransferStateModule?

This comment has been minimized.

Copy link
@vikerman

vikerman Sep 13, 2017

Author Contributor

Renaming all Store to State

@alxhub

alxhub approved these changes Sep 13, 2017

@mary-poppins

This comment has been minimized.

Copy link

commented Sep 13, 2017

@mary-poppins

This comment has been minimized.

Copy link

commented Sep 13, 2017

@mary-poppins

This comment has been minimized.

Copy link

commented Sep 13, 2017

export {By} from './dom/debug/by';
export {DOCUMENT} from './dom/dom_tokens';
export {EVENT_MANAGER_PLUGINS, EventManager} from './dom/events/event_manager';
export {HAMMER_GESTURE_CONFIG, HammerGestureConfig} from './dom/events/hammer_gestures';
export {DomSanitizer, SafeHtml, SafeResourceUrl, SafeScript, SafeStyle, SafeUrl, SafeValue} from './security/dom_sanitization_service';

This comment has been minimized.

Copy link
@mhevery

mhevery Sep 13, 2017

Member

revert

*
* @experimental
*/
export interface StateKey<T> { readonly key: string; }

This comment has been minimized.

Copy link
@mhevery

mhevery Sep 13, 2017

Member

brandeed makeKey

@mary-poppins

This comment has been minimized.

Copy link

commented Sep 14, 2017

@mary-poppins

This comment has been minimized.

Copy link

commented Sep 14, 2017

@mary-poppins

This comment has been minimized.

Copy link

commented Sep 14, 2017

@mary-poppins

This comment has been minimized.

Copy link

commented Sep 14, 2017

@mary-poppins

This comment has been minimized.

Copy link

commented Sep 14, 2017

@mary-poppins

This comment has been minimized.

Copy link

commented Sep 14, 2017

@mary-poppins

This comment has been minimized.

Copy link

commented Sep 19, 2017

@IgorMinar

This comment has been minimized.

Copy link
Member

commented Sep 20, 2017

can you please rebase?

feat(platform-server): add an API to transfer state from server
TransferState provides a shared store that is transferred from the
server to client. To use it import BrowserTransferStateModule from the
client app module and ServerTransferStateModule from the server app
module and TransferState will be available as an Injectable object.
@mary-poppins

This comment has been minimized.

Copy link

commented Sep 21, 2017

@mcferren

This comment has been minimized.

Copy link

commented Sep 30, 2017

can you please share a link to an example application that uses this feature?

@feloy

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2017

@mcferren

This comment has been minimized.

Copy link

commented Oct 13, 2017

Thanks @feloy

but I am consistently getting the default null value when I console logging the this.transferState.get(BATCH_KEY, null);

my use case is not tied to resolving a route as you have implemented. You used onSerialize(key: StateKey, callback: () => T): void as a result of resolving a route?

is this.transferState.set(BATCH_KEY, batch); currently operational?

i have two functions in my injectable. One that awaits an ajax call then sets. And another function that gets and returns. Inside my Injectable file, above the @Injectable syntax, I define
const BATCH_KEY = makeStateKey('batch');

my component checks and calls the injectable function that sets if( !isPlatformBrowser( this.platformId ) ) { and it calls the injectable function that gets if( isPlatformBrowser( this.platformId ) ) {

what am I doing wrong to consistently get the default null value?

@dottodot

This comment has been minimized.

Copy link

commented Oct 15, 2017

@feloy I've tried to follow your article but const found = this.transferState.hasKey(RESULT_KEY); always returns false on the browser, making the transferState to not work.

Also this.transferState.get(RESULT_KEY, null); returns null

I've even cloned your repo and that's the same.

Oddly though if I do console.log(this.transferState); at the beginning of my resolve it shows the data is in the store. It's as if get and hasKey don't work.

@feloy

This comment has been minimized.

Copy link
Contributor

commented Oct 15, 2017

@dottodot @mcferren Have you tried with different browsers?

Unfortunately, on a Linux system, I can make it work with Firefox only.

@dottodot

This comment has been minimized.

Copy link

commented Oct 15, 2017

On mac just tried Chrome, Safari and Firefox and none of them work.

@dottodot

This comment has been minimized.

Copy link

commented Oct 18, 2017

I've figured it out. In my main.ts I had

platformBrowserDynamic()
  .bootstrapModule(AppBrowserModule)
  .catch(err => console.log(err));

but you should bootstrap the application only when the document is ready so like this.

document.addEventListener('DOMContentLoaded', () => {
  platformBrowserDynamic()
    .bootstrapModule(AppBrowserModule)
    .catch(err => console.log(err));
});

now the transferState works

@mcferren

This comment has been minimized.

Copy link

commented Oct 19, 2017

yes I can confirm this as well - thank you fellas

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.