Navigation Menu

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(interruptsource): allow interrupts to use an ssr option #130

Closed
wants to merge 3 commits into from

Conversation

moribvndvs
Copy link
Owner

@moribvndvs moribvndvs commented Jul 24, 2019

ssr option instructs interrupt sources to ignore targets
derived from global context such as window or document so
they can be safely used in ssr/universal apps.

If you are using ssr/universal, do not use
DEFAULT_INTERRUPTSOURCES. Instead, use
createDefaultInterruptSources({ssr: !isPlatformBrowser(platformId) }).
See docs or https://stackoverflow.com/a/46893433/64750
for information on how to use isPlatformBrowser.

Fixes #77, #115

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x")

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior? (You can also link to an open issue here)

No support for SSR/Universal apps.

What is the new behavior?
Adds an opt-in option when creating interrupt sources that will allow compatibility with SSR/universal.

Does this PR introduce a breaking change? (check one with "x")

[ ] Yes
[x] No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

ssr option instructs interrupt sources to ignore targets
derived from global context such as window or document so
they can be safely used in ssr/universal apps.

If you are using ssr/universal, do not use
DEFAULT_INTERRUPTSOURCES. Instead, use
`createDefaultInterruptSources({ssr: !isPlatformBrowser(platformId) })`.
 See docs or https://stackoverflow.com/a/46893433/64750
for information on how to use isPlatformBrowser.

Fixes #77, #115
@moribvndvs moribvndvs self-assigned this Jul 24, 2019
@moribvndvs moribvndvs added the bug label Jul 24, 2019
@moribvndvs moribvndvs added this to In progress in 8.0.0 Release via automation Jul 24, 2019
@coveralls
Copy link

coveralls commented Jul 24, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling ca8d68b on ssr into db0148b on master.

8.0.0 Release automation moved this from In progress to Review in progress Jul 26, 2019
const target =
options && (options as EventTargetInterruptOptions).ssr
? null
: document.documentElement;
Copy link

Choose a reason for hiding this comment

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

I'm still getting the below compile error when I import NgIdleKeepaliveModule.forRoot() in the app.module of my Angular project, and run the command npm run build:ssr && npm run serve:ssr.

/Users/bfwg/code/ng2-idle-example/dist/server/main.js:112377
            : document.documentElement;
              ^

ReferenceError: document is not defined
    at new DocumentInterruptSource (/Users/bfwg/code/ng2-idle-example/dist/server/main.js:112377:15)
    at createDefaultInterruptSources (/Users/bfwg/code/ng2-idle-example/dist/server/main.js:112522:9)
    at Module.evxF (/Users/bfwg/code/ng2-idle-example/dist/server/main.js:112527:34)
    at __webpack_require__ (/Users/bfwg/code/ng2-idle-example/dist/server/main.js:20:30)
    at Object.V7fC (/Users/bfwg/code/ng2-idle-example/dist/server/main.js:97581:13)
    at __webpack_require__ (/Users/bfwg/code/ng2-idle-example/dist/server/main.js:20:30)
    at Object.K011 (/Users/bfwg/code/ng2-idle-example/dist/server/main.js:86964:37)
    at __webpack_require__ (/Users/bfwg/code/ng2-idle-example/dist/server/main.js:20:30)
    at Object.0 (/Users/bfwg/code/ng2-idle-example/dist/server/main.js:437:18)
    at __webpack_require__ (/Users/bfwg/code/ng2-idle-example/dist/server/main.js:20:30)

main.js:112377 points to this line.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Can you publish this example code somewhere?

Copy link

Choose a reason for hiding this comment

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

I made a ssr boilerplate app https://github.com/bfwg/ng2-idle-ssr-example.
Steps to reproduce:

  1. clone the ng2-idle-ssr-example app.
  2. npm install
  3. build core and keepalive module from thie PR.
  4. cope them into the node_modules of the boilerplate app, inside @ng-idle
  5. run npm run build:ssr && npm run serve:ssr command.

Please let me know if you need more information.

Choose a reason for hiding this comment

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

@bfwg, did you find a solution to your problem? I am having similar errors to yours.

README.md Outdated Show resolved Hide resolved
@moribvndvs moribvndvs closed this Sep 30, 2021
8.0.0 Release automation moved this from Review in progress to Done Sep 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
8.0.0 Release
  
Done
Development

Successfully merging this pull request may close these issues.

ng2-idle server pre rendering issue
4 participants