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

Next13: unwanted div element added inside layout #42345

Closed
1 task done
mehrdad-shokri opened this issue Nov 2, 2022 · 10 comments
Closed
1 task done

Next13: unwanted div element added inside layout #42345

mehrdad-shokri opened this issue Nov 2, 2022 · 10 comments
Labels
area: app App directory (appDir: true) bug Issue was opened via the bug report template.

Comments

@mehrdad-shokri
Copy link

mehrdad-shokri commented Nov 2, 2022

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 22.1.0: Sun Oct  9 20:14:30 PDT 2022; root:xnu-8792.41.9~2/RELEASE_ARM64_T8103
Binaries:
  Node: 16.17.1
  npm: 8.15.0
  Yarn: 1.22.19
  pnpm: N/A
Relevant packages:
  next: 13.0.0
  eslint-config-next: 13.0.0
  react: 18.2.0
  react-dom: 18.2.0

warn - Latest canary version not detected, detected: "13.0.0", newest: "13.0.2-canary.0".
Please try the latest canary version (npm install next@canary) to confirm the issue still exists before creating a new issue.
Read more - https://nextjs.org/docs/messages/opening-an-issue

What browser are you using? (if relevant)

Chrome

How are you deploying your application? (if relevant)

No response

Describe the Bug

I can confirm that for every nested layout that I've used, there is a parent div which children is wrapped inside them. I've checked my code I don't return that <div> so I suspect is wrapping children around another div.

Expected Behavior

I expect it to be wrapped inside Fragment(if necessary) because,

messes my styles

Link to reproduction

yarn create next-app nextjs-server-components --experimental-app

To Reproduce

Check the default Next.js rendered HTML for path /, there is a <div> around page container which is neither present inside the default layout nor inside the page

@mehrdad-shokri mehrdad-shokri added the bug Issue was opened via the bug report template. label Nov 2, 2022
@mehrdad-shokri
Copy link
Author

After upgrading to next 13.0.1 this is the div which gets added inside layouts: <div data-nextjs-scroll-focus-boundary I don't know what its use is but it messes my styles. Seriously, we though our pages is gonna mount inside layouts and adding another intermediate <div> between these can mess a lot of things.

@mehrdad-shokri
Copy link
Author

I've put a video to show what I mean but adding intermediate div elements between layout and pages causes to not be able to use nested layouts since you can't style the nested layout because it's gonna be rendered inside a bunch of other divs and what you meant to be placed in place of the children would end of inside a bunch of divs so you won't be able to style the nested layout
https://user-images.githubusercontent.com/13661520/199484479-71b0696d-eff8-44a4-8433-b1bbd99168cb.mov

@HaNdTriX
Copy link
Contributor

HaNdTriX commented Nov 2, 2022

This is a known Issue and is being discussed in #41745 (reply in thread)

Background: If you navigate inside nested layouts, Next.js needs to restore the scroll position or scroll the loaded element into view. Nevertheless React currently has no API to select the DOMNodes of a React subtree (inside strict mode), because a subtree could be a string or even a boolean. So the current solution is to wrap the subtree inside a div to ensure we can scroll the new element into view.

Afaik the React Team and Next.js Team is aware of this issue. Hopefully it will be resolved in the future.

@mehrdad-shokri
Copy link
Author

@HaNdTriX couldn't Nextjs register that ref on the first child of the children? is that possible?

@HaNdTriX
Copy link
Contributor

HaNdTriX commented Nov 2, 2022

If that component has a DOM Node to reference and if React would expose an API like findDOMNode that might be possible.

Please note that this issue is not trivial, because of the way the DOM works. JSX supports all kinds of Node DOMNode like React Components (strings, booleans, arrays, undefined, etc.). These component results are currently not referenceable using traditional DOM APIs.

function MyComponent() {
  return <>I am a string, you can\'t reference me via traditions DOM APIs</>
}

The DOM only allows us to query components in a performant way. If you want to query a specific TextNode (e.g. 3rd word in a sentence) or an HTML comment things get complicated and slow.

Disclaimer: I am not part of Next.js Core Team

@mehrdad-shokri
Copy link
Author

mehrdad-shokri commented Nov 2, 2022

How about this document.getElementById("childrenWrapper").firstChild. I think in case of Fragments, boolean, string, undefined, null and array(basically where firstChild is null) we can register the ref on the childWrapper itself. so we can have a common ID that we can wrap children inside that ID and Next.js can handle it this way without the need to add another HTML element

@long-lazuli
Copy link

long-lazuli commented Nov 6, 2022

Hi,
While waiting for a proper solution, I wanted to mention that this simple CSS resolve a lot of my issues with these unwanted div.

[data-nextjs-scroll-focus-boundary] {
  display: contents;
}

it will not work for older browsers unfortunately.

@timneutkens timneutkens added the area: app App directory (appDir: true) label Nov 6, 2022
@timneutkens
Copy link
Member

I'm going to close this issue as this is behaving as intended for right now. As said in #41745 (reply in thread) we're planning to remove it in the near future.

@danieljpgo
Copy link

It was fixed in the last canary

#43717

@github-actions
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: app App directory (appDir: true) bug Issue was opened via the bug report template.
Projects
None yet
Development

No branches or pull requests

5 participants