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

refactor: introduce SSR variable for platform checks #1080

Merged
merged 3 commits into from
Jul 19, 2022

Conversation

dhhyi
Copy link
Collaborator

@dhhyi dhhyi commented Mar 22, 2022

PR Type

[x] Feature
[x] Refactoring (no functional changes, no API changes)

What Is the Current Behavior?

For checking if the current Angular app is running in SSR or Browser, the PLATFORM_ID has to be injected and the methods isPlatformBrowser and isPlatformServer have to be used to perform the check. This always requires DI context and is rather verbose.

What Is the New Behavior?

  • The customized build sets the variable SSR that can be used for this check.
  • Added lint rule that suggests code changes.
  • Transformed all occurrences.
  • Add onSSREnvironment to jest describe so the SSR variable can be set for testing (default is false)

Does this PR Introduce a Breaking Change?

[ ] Yes
[x] No

Other Information

As the SSR value is now injected into the build process with the webpack DefinePlugin, the values can be used for code minification. When using the usual way with PLATFORM_ID, the chack can only be performed at runtime and all code is ending up in server and client bundles.

Verify this by adding the following code:

    if (!SSR) {
      console.log('browser - QWERTY1234');
    }
    if (SSR) {
      console.log('server - QWERTY1234');
    }

Check where it turns up with (bash):

~$ npm run build
...
~$ grep -ro '\w* - QWERTY1234' dist
dist/server/main.js:server - QWERTY1234
dist/browser/main.e9860fd746f0e9c6.js:browser - QWERTY1234

AB#75426

@dhhyi dhhyi added refactoring Refactoring of current code community Community contributions labels Mar 22, 2022
@dhhyi dhhyi force-pushed the refactor/variable-for-platform-id branch 3 times, most recently from f4754bf to 8245142 Compare March 31, 2022 08:03
@shauke shauke added the vote Open for community input (pro/contra) label Apr 1, 2022
@dhhyi dhhyi force-pushed the refactor/variable-for-platform-id branch 2 times, most recently from 236b3b8 to b83c9dd Compare April 3, 2022 11:52
@dhhyi dhhyi force-pushed the refactor/variable-for-platform-id branch from b83c9dd to 48a8305 Compare April 17, 2022 20:35
@dhhyi dhhyi force-pushed the refactor/variable-for-platform-id branch from 48a8305 to a02d847 Compare April 21, 2022 14:26
@dhhyi dhhyi force-pushed the refactor/variable-for-platform-id branch from a02d847 to 20b86e5 Compare May 2, 2022 07:43
@shauke shauke added this to the 3.0 milestone May 2, 2022
@shauke
Copy link
Collaborator

shauke commented May 2, 2022

It seems like a nice improvement but we should introduce it with the next major version and not with a 2.x minor version. At least if there are no further objections.

@dhhyi
Copy link
Collaborator Author

dhhyi commented May 2, 2022

It seems like a nice improvement but we should introduce it with the next major version and not with a 2.x minor version.

Good idea. I'll keep the branch up-to-date until then whenever it has conflicts 😉👍

@dhhyi dhhyi force-pushed the refactor/variable-for-platform-id branch 2 times, most recently from f1511da to 160e531 Compare May 20, 2022 12:15
@dhhyi dhhyi force-pushed the refactor/variable-for-platform-id branch from add309b to c3cf255 Compare June 6, 2022 09:22
@dhhyi dhhyi force-pushed the refactor/variable-for-platform-id branch from c3cf255 to 02bc5ee Compare June 21, 2022 15:32
@dhhyi dhhyi force-pushed the refactor/variable-for-platform-id branch from 02bc5ee to 18a00ce Compare June 27, 2022 14:06
@dhhyi dhhyi force-pushed the refactor/variable-for-platform-id branch from 18a00ce to 6b9929d Compare July 12, 2022 14:21
@shauke shauke merged commit 7c6a226 into develop Jul 19, 2022
@shauke shauke deleted the refactor/variable-for-platform-id branch July 19, 2022 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Community contributions refactoring Refactoring of current code vote Open for community input (pro/contra)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants