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/Tests] verify components in SSR #10237

Open
maxpatiiuk opened this issue Sep 6, 2024 · 2 comments
Open

[Refactor/Tests] verify components in SSR #10237

maxpatiiuk opened this issue Sep 6, 2024 · 2 comments
Labels
0 - new New issues that need assignment. p - medium Issue is non core or affecting less that 60% of people using the library testing Issues related to automated or manual testing.

Comments

@maxpatiiuk
Copy link
Member

maxpatiiuk commented Sep 6, 2024

Priority impact

p - medium

Test type

SSR

Which Component(s)

all

Unstable Tests

[not actionable before the migration]

When Stencil component is rendered in the Hydrate build, Stencil was providing mocks for a lot of dom APIs. (source code)

When Lit elements are rendered in SSR, Lit also provides a mock dom API, but it's much more lightweight (source code 1, source code 2)

That means that if before Stencil would mock the element.closest() calls in SSR, these are no longer mocked in Lit. This may cause some Calcite components to start failing in SSR after migration (with errors like element.closest is not a function)

Options:

  • After the migration, manually test each Calcite component in SSR
  • After the migration, setup automated SSR test for each component
  • Look for an alternative DOM API shim (or maybe continue using Stencil's shim if that is possible) so that most DOM APIs are still covered. Cons: complexity and slower SSR
  • If you really need some DOM APIs in Node.js, the Lit team would likely be open to contributions

When you identify a place where a DOM API that is not available in SSR is accessed, you have a few options:

  • Use optional chaining for property/methods:

    - const parent = element.closest("calcite-tree");
    + const parent = element.closest?.("calcite-tree");
  • Use globalThis with optional chaining for global APIs:

    - const userAgent = navigator.userAgent;
    + const userAgent = globalThis.navigator?.userAgent;
  • Conditionally execute code in browser environment only:

    import {isServer} from 'lit';
    
    if (isServer) {
      // only runs in server environments like Node
    } else {
      // runs in the browser
    }
    • I know that Calcite had issues in the past with the isBrowser check that Stencil provided. The Stencil check was based on presence/absence of DOM globals like window.navigator and etc. Lit's check instead is determined at build-time. That works thanks to a feature called export conditions (this let's Lit set that variable to true in the node.js build, and false in the browser build)
  • Tell consumers of Calcite components in SSR that they need to have happy-dom or jsdom environment setup to correctly render Calcite components.

Lit docs:

Test error, if applicable

No response

PR skipped, if applicable

No response

Additional Info

No response

@maxpatiiuk maxpatiiuk added 0 - new New issues that need assignment. needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. testing Issues related to automated or manual testing. labels Sep 6, 2024
@maxpatiiuk maxpatiiuk changed the title [Migration/Tests] verify components in SSR [Refactor/Tests] verify components in SSR Sep 12, 2024
@github-actions github-actions bot added the p - medium Issue is non core or affecting less that 60% of people using the library label Sep 12, 2024
@jcfranco jcfranco added the blocked This issue is blocked by another issue. label Sep 27, 2024
@jcfranco
Copy link
Member

Blocked by #10310.

Copy link
Contributor

Issue #10310 has been closed, this issue is ready for re-evaluation.

cc @geospatialem,@DitwanP

@github-actions github-actions bot removed the blocked This issue is blocked by another issue. label Nov 26, 2024
@geospatialem geospatialem removed the needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. label Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 - new New issues that need assignment. p - medium Issue is non core or affecting less that 60% of people using the library testing Issues related to automated or manual testing.
Projects
None yet
Development

No branches or pull requests

3 participants