Skip to content
This repository was archived by the owner on Nov 22, 2024. It is now read-only.

Proposal: window service #828

Closed
Toxicable opened this issue Oct 17, 2017 · 8 comments
Closed

Proposal: window service #828

Toxicable opened this issue Oct 17, 2017 · 8 comments
Milestone

Comments

@Toxicable
Copy link

TLDR: A pluggable service that provides a consistent pattern for providing services that usually exist on window across platforms that are Universal compatible.

API

interface WindowService {
  localStorage: Storage;
  sessionStorage: Storage;
  caches: CacheStorage;
  ...//other relevant apis
}

//modules
BrowserWindowServiceModule
ServerNoopWindowServiceModule

We'll provide a module that provides services that noop out the api's such as localstorage since it doesn't make any sense for that to work on the server, but in a way that it will be Universal compatible.

Pluggable

You can provide your own implementations for the services found inside the WindowService like so

{
  provide: LOCAL_STORAGE_BACKEND,
  useFactory: myServerLocalStorageBackendFactory
}

draft imp here https://github.com/Toxicable/universal/blob/local-storage/modules/common/src/local-storage/local-storage.ts

@CaerusKaru
Copy link
Member

Wouldn't it be better to just provide a Window implementation, like @vikerman did for DOCUMENT? The only issue/PR I could find in angular/angular was angular/angular#12694, and the main detraction was lack of portability to node. However, just doing a quick search pulls up jsdom as a possible reference implementation (similar to how we use domino for Document).

It seems better to provide a catch-all implementation than to patch certain services. It would also encourage Angular users to avoid using window directly, which may/may not be an Angular anti-pattern.

@Toxicable
Copy link
Author

jsdom would be a replacement for domino since it does the whole DOM, therefore that's not an option.
I already encourage users to not use window directly since it can often lead to unexpected behaviour, such as Universal incompatibility, but with this they can use it without such issues.

@CaerusKaru
Copy link
Member

We wouldn't use jsdom explicitly, we could cherry-pick the higher-level options, and then use domino as the document property.

@Toxicable
Copy link
Author

Toxicable commented Oct 17, 2017

I havn't look at their imp but how would things like localStorage and other storage API's work? (the original reason for this).
The idea with this is to provide a consistent pattern for working with these API's that allows for users to easily plug in their own imps for specific API's without having to bloat their client side bundles or have platform checks everywhere. With default just being "it'll work, but do nothing", which is 100% fine for most cases i've heard so far

@CaerusKaru
Copy link
Member

jsdom doesn't have LocalStorage, but there are other implementations around that we could patch this with, e.g. node-localstorage. My general suggestion, though, is that we shouldn't have an "a la carte" option, it should just be a catch-all.

@Ks89
Copy link

Ks89 commented Nov 13, 2017

I found this issue thanks to this one: angular/universal-starter#189
I have a problem while trying to use hammerjs with SSR.
I tried to add it to the official angular-universal starter and the result is:

> [email protected] serve:ssr /Users/ks89/git/universal-starter
> node dist/server

/Users/ks89/git/universal-starter/dist/server.js:126460
})(window, document, 'Hammer');
   ^
ReferenceError: window is not defined
    at Object._MAP.8 (/Users/ks89/git/universal-starter/dist/server.js:126460:4)
    at __webpack_require__ (/Users/ks89/git/universal-starter/dist/server.js:20:30)
    at Object.LYNW (/Users/ks89/git/universal-starter/dist/server.js:105040:8254)
    at n (/Users/ks89/git/universal-starter/dist/server.js:105040:149)
    at Object.aR8+ (/Users/ks89/git/universal-starter/dist/server.js:105040:10320)
    at n (/Users/ks89/git/universal-starter/dist/server.js:105040:149)
    at Object.bNRb (/Users/ks89/git/universal-starter/dist/server.js:105040:10973)
    at n (/Users/ks89/git/universal-starter/dist/server.js:105040:149)
    at Object.JwoV (/Users/ks89/git/universal-starter/dist/server.js:105040:1956)
    at n (/Users/ks89/git/universal-starter/dist/server.js:105040:149)

edit: oh wait. I imported hammerjs in main.ts and now it is working.

@CaerusKaru CaerusKaru added this to the Backlog milestone Mar 7, 2018
@Toxicable
Copy link
Author

superseeded by #992

coltenkrauter pushed a commit to coltenkrauter/angular-ssr-example that referenced this issue Aug 2, 2019
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants