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.

Sign up for GitHub

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

Jump to bottom

Initial render issues due to dynamic import of custom elements in calcite-components-react #8143

Closed
2 of 5 tasks
benelan opened this issue Nov 8, 2023 · 2 comments
Closed
2 of 5 tasks

Initial render issues due to dynamic import of custom elements in calcite-components-react #8143

benelan opened this issue Nov 8, 2023 · 2 comments
Assignees
@geospatialem @DitwanP
Labels
4 - verified Issues that have been released and confirmed resolved. breaking change Issues and pull requests with code changes that are not backwards compatible. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. Calcite (dev) Issues logged by Calcite developers. calcite-components-react Issues specific to the @esri/calcite-components-react package.
Milestone
2023 November Pri...

Comments

@benelan
Copy link
Member

benelan commented Nov 8, 2023 •
edited by eriklharper
Loading

Check existing issues

Actual Behavior

Some initial render issues have popped up that I traced back to the patch created in #7521. The patch uses a dynamic import to ensure the custom elements are not defined on the server, which causes errors. This may be an issue on Stencil's end, where they need to force a VDOM rerender when the components are hydrated.

The current workaround is to import/define the custom elements in addition to the react components. This was the only possible behavior until @esri/calcite-components-react@v1.5.0, when we enabled includeImportCustomElements. See #7185 for examples.

Expected Behavior

I expect one of the following:

  1. No initial render issues when taking advantage of includeImportCustomElements. This means we would likely need to remove the patch and find a new solution for the SSR build errors brought up in Calcite React tests fail in vitest #7486 and Error using calcite components in Gatsby site during server-side rendering #7493. We should do additional testing after build(deps)!: bump Stencil to v4 #8108 is installed, because Stencil may have resolved the issues on their end in the new versions.
  2. Disable includeImportCustomElements to prevent confusion and buggy behavior. This would be a breaking change.

Reproduction Sample

https://codesandbox.io/p/sandbox/calcite-components-react-hm5rwn?file=%2Fsrc%2FApp.tsx%3A5%2C74

Reproduction Steps

  1. The current state of the sample works properly
  2. Remove lines 2-5 in App.tsx
  3. Notice the slider appears to have a value of 0 even though the internal state is 50. The banana icon at the end of the title also didn't render initially.
  4. Move the slider or click the "Clear" button, and notice the icon appears and the slider starts behaving normally.

Reproduction Version

1.10.0

Relevant Info

No response

Regression?

No response

Priority impact

p3 - want for upcoming milestone

Impact

No response

Calcite package

  • @esri/calcite-components
  • @esri/calcite-components-react
  • @esri/calcite-design-tokens
  • @esri/eslint-plugin-calcite-components

Esri team

Calcite (dev)
@benelan benelan added bug Bug reports for broken functionality. Issues should include a reproduction of the bug. 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Nov 8, 2023
@github-actions github-actions bot added Calcite (dev) Issues logged by Calcite developers. p3 - want for upcoming milestone calcite-components-react Issues specific to the @esri/calcite-components-react package. labels Nov 8, 2023
@benelan benelan added 2 - in development Issues that are actively being worked on. and removed 0 - new New issues that need assignment. labels Nov 15, 2023
@benelan benelan added this to the 2023 November Priorities milestone Nov 15, 2023
@benelan benelan added the breaking change Issues and pull requests with code changes that are not backwards compatible. label Nov 21, 2023
@benelan benelan mentioned this issue Nov 22, 2023
Merged
benelan added a commit that referenced this issue Nov 22, 2023
@benelan
fix(react)!: disable includeImportCustomElements to resolve initial r…
cc72d60 
…ender issues (#8248)

**Related Issue:** #8143

## Summary

The `ReferenceError: navigator is not defined` error is still present in
Stencil v4 when removing the patch. Disabled
[`includeImportCustomElements`](https://stenciljs.com/docs/react#includeimportcustomelements)
for now while we investigate an alternative fix that doesn't cause
initial render issues.


BREAKING CHANGE: Disabled `includeImportCustomElements`. Make sure to
import components from `@esri/calcite-components` in addition to the
react wrappers. For example, the first code snippet in #7185 is now
required, or else the custom elements will not be defined on the window.
benelan added a commit that referenced this issue Nov 23, 2023
@benelan
fix(react)!: disable includeImportCustomElements to resolve initial r…
d761007 
…ender issues (#8248)

**Related Issue:** #8143

## Summary

The `ReferenceError: navigator is not defined` error is still present in
Stencil v4 when removing the patch. Disabled
[`includeImportCustomElements`](https://stenciljs.com/docs/react#includeimportcustomelements)
for now while we investigate an alternative fix that doesn't cause
initial render issues.


BREAKING CHANGE: Disabled `includeImportCustomElements`. Make sure to
import components from `@esri/calcite-components` in addition to the
react wrappers. For example, the first code snippet in #7185 is now
required, or else the custom elements will not be defined on the window.
@geospatialem geospatialem added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 2 - in development Issues that are actively being worked on. needs triage Planning workflow - pending design/dev review. labels Nov 27, 2023
@github-actions github-actions bot assigned geospatialem and DitwanP and unassigned benelan Nov 27, 2023
@github-actions GitHub Actions
Copy link
Contributor

Installed and assigned for verification.

benelan added a commit that referenced this issue Nov 29, 2023
@benelan
fix(react)!: disable includeImportCustomElements to resolve initial r…
5fca601 
…ender issues (#8248)

**Related Issue:** #8143

## Summary

The `ReferenceError: navigator is not defined` error is still present in
Stencil v4 when removing the patch. Disabled
[`includeImportCustomElements`](https://stenciljs.com/docs/react#includeimportcustomelements)
for now while we investigate an alternative fix that doesn't cause
initial render issues.


BREAKING CHANGE: Disabled `includeImportCustomElements`. Make sure to
import components from `@esri/calcite-components` in addition to the
react wrappers. For example, the first code snippet in #7185 is now
required, or else the custom elements will not be defined on the window.
@DitwanP
Copy link
Contributor

DitwanP commented Nov 29, 2023

Verified on code sandbox

@DitwanP DitwanP closed this as completed Nov 29, 2023
@DitwanP DitwanP mentioned this issue Nov 29, 2023
Closed
4 tasks
@DitwanP DitwanP added 4 - verified Issues that have been released and confirmed resolved. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Nov 29, 2023
benelan added a commit that referenced this issue Dec 1, 2023
@benelan
fix(react)!: disable includeImportCustomElements to resolve initial r…
0de1d1f 
…ender issues (#8248)

**Related Issue:** #8143

## Summary

The `ReferenceError: navigator is not defined` error is still present in
Stencil v4 when removing the patch. Disabled
[`includeImportCustomElements`](https://stenciljs.com/docs/react#includeimportcustomelements)
for now while we investigate an alternative fix that doesn't cause
initial render issues.


BREAKING CHANGE: Disabled `includeImportCustomElements`. Make sure to
import components from `@esri/calcite-components` in addition to the
react wrappers. For example, the first code snippet in #7185 is now
required, or else the custom elements will not be defined on the window.
benelan added a commit that referenced this issue Dec 1, 2023
@benelan
fix(react)!: disable includeImportCustomElements to resolve initial r…
843821f 
…ender issues (#8248)

**Related Issue:** #8143

## Summary

The `ReferenceError: navigator is not defined` error is still present in
Stencil v4 when removing the patch. Disabled
[`includeImportCustomElements`](https://stenciljs.com/docs/react#includeimportcustomelements)
for now while we investigate an alternative fix that doesn't cause
initial render issues.


BREAKING CHANGE: Disabled `includeImportCustomElements`. Make sure to
import components from `@esri/calcite-components` in addition to the
react wrappers. For example, the first code snippet in #7185 is now
required, or else the custom elements will not be defined on the window.
benelan added a commit that referenced this issue Dec 2, 2023
@benelan
fix(react)!: disable includeImportCustomElements to resolve initial r…
0948c1a 
…ender issues (#8248)

**Related Issue:** #8143

## Summary

The `ReferenceError: navigator is not defined` error is still present in
Stencil v4 when removing the patch. Disabled
[`includeImportCustomElements`](https://stenciljs.com/docs/react#includeimportcustomelements)
for now while we investigate an alternative fix that doesn't cause
initial render issues.


BREAKING CHANGE: Disabled `includeImportCustomElements`. Make sure to
import components from `@esri/calcite-components` in addition to the
react wrappers. For example, the first code snippet in #7185 is now
required, or else the custom elements will not be defined on the window.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@geospatialem geospatialem

@DitwanP DitwanP

Labels
4 - verified Issues that have been released and confirmed resolved. breaking change Issues and pull requests with code changes that are not backwards compatible. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. Calcite (dev) Issues logged by Calcite developers. calcite-components-react Issues specific to the @esri/calcite-components-react package.
Projects
None yet
Milestone
2023 November Priorities
Development

No branches or pull requests
3 participants
@geospatialem @benelan @DitwanP