fix(env): render span for env detection in portal

[yukukotani]

Closes #6374

⛳️ Current behavior (updates)

Please describe the current behavior that you are modifying

span is rendered next to the children of ChakraProvider. This breaks some tests to check DOM structure.

🚀 New behavior

Please describe the behavior or changes this PR adds

span is rendered in the portal, so DOM structure of the component isn't affected.

💣 Is this a breaking change (Yes/No):

No

📝 Additional Information

It seems that we can skip rendering span if environmentProp is given. I'll work on that in separate PR because it's not related to the issue.

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: 42ffb1db29afaf6128757d9903830ef32b5b924e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages

Name	Type
@chakra-ui/react-env	Minor
@chakra-ui/media-query	Patch
@chakra-ui/provider	Patch
@chakra-ui/react	Patch
@chakra-ui/skeleton	Patch
@chakra-ui/test-utils	Patch
create-react-app-ts	Patch
gatsby-starter-default	Patch
chakra-nextjs	Patch
chakra-nextjs-ts	Patch
@chakra-ui/props-docs	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
chakra-ui-storybook	✅ Ready (Inspect)	Visit Preview	Aug 10, 2022 at 5:50AM (UTC)

[codesandbox-ci[bot]]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 42ffb1db29afaf6128757d9903830ef32b5b924e:

Sandbox	Source
create-react-app-ts	Configuration
ChakraProvider empty span issue	Issue #6374

[segunadebayo]

@yukukotani Kindly resolve the conflicts in this PR. I'd like to get this merged.

[yukukotani]

@segunadebayo Resolved. But lock file has large diff because of pnpm's version update. Is it ok?

[segunadebayo]

Thinking about this again. It seems to me that using a portal defeats the purpose of the env in the first place.

Its purpose is to get the correct reference to window or document regardless of the environment used. Portal will append to the document.body by default. The document might be incorrect in most cases.

The best way to test this is to render any component that uses the env within an iframe (using react-frame-component) and see if it still works as intended.

[yukukotani]

@segunadebayo I think this works because Portal also detects env by tempNode.

Anyway, we need tests for the env but it's not easy since jsdom doesn't support iframe's srcdoc. react-frame-component doesn't work with Jest + jsdom.

e2e testing is one of the options.

[ksocha]

That might be a dumb proposition, but what if we just could prevent the span element being rendered after the node is set, like so:

- {mounted && (
+ {mounted && !node && (
    <span
      ref={(el) => {
        startTransition(() => {
          if (el) setNode(el)
        })
      }}
    />
  )}

[segunadebayo]

So far, setting the span to be hidden has worked, and I would leave it as is for now.

I'll use a Portal as a fallback implementation if this becomes an issue.

Thanks for your efforts on this one @yukukotani