vercel / next.js

The React Framework
https://nextjs.org
MIT License
126.78k stars 26.95k forks source link

ref doesn't work with dynamically imported components #4957

Closed juancampa closed 6 years ago

juancampa commented 6 years ago

Bug report

Describe the bug

When a component is wrapped in next/dynamic the passed ref should be a ref to the imported component, not an instance of NoSsr or LoadableComponent

To Reproduce

  1. Import a component using next/dynamic
  2. Use said component in render passing a ref (use a function to received the ref)

Result: the function is called with either a NoSSR component or a LoadableComponent instead of an instance of the component imported in step 1

Expected behavior

The function gets called with an instance of the dynamically loaded component

System information

Additional context

Didn't try with createRef but it shouldn't make a difference

juancampa commented 6 years ago

This was reported in #2842 (closed) but since there's a new API for forwarding refs I believe it should be reconsidered

timneutkens commented 6 years ago

Please create a minimal reproducible example in a github repo so it's easier to reproduce.

juancampa commented 6 years ago

Sure, here it is:

https://github.com/juancampa/next-bug/blob/master/pages/index.js

timneutkens commented 6 years ago

Expecting this to be one of the cases for the new createRef API

HaNdTriX commented 6 years ago

Solution

@juancampa You can use the forwardRef api to implement it yourself.

pages/index.js

import React from 'react'
import dynamic from 'next/dynamic';

// Dynamically imported component
const Component = dynamic(import('../components/Component'));
const ForwardedRefComponent = React.forwardRef((props, ref) => (
  <Component {...props} forwardedRef={ref} />
))

export default class Index extends React.Component {
  constructor(props) {
    super(props)
    this.myRef = React.createRef();
  }

  componentDidMount() {
    console.log('this.myRef', this.myRef)
  }

  render() {
    return <ForwardedRefComponent ref={this.myRef}/>;
  }
}

components/Component.js

export default ({ forwardedRef }) => (
  <div ref={forwardedRef}>
    Hi from Component
  </div>
)

Please note: You can also omit the forwardRef api and pass the ref as forwardedRef in directly.

Example

Issues adding this functionality to next.js

I don't think it makes sense to implement it in next now since:

timneutkens commented 6 years ago

Thanks @HaNdTriX!

mohsinulhaq commented 6 years ago

@HaNdTriX what if we want to set ref on the Component component itself?

timneutkens commented 6 years ago

Going to close this as the issue has been answered and @HaNdTriX's reasoning is correct.

muhaimincs commented 5 years ago

is this going to be official workaround?

phattranky commented 4 years ago

@HaNdTriX what if we want to set ref on the Component component itself?

Then, you can simply user other Component wrap all component using ref. Then use dynamic on WrapperComponent. So the child component will use like usual

xfause commented 4 years ago

Solution

@juancampa You can use the forwardRef api to implement it yourself.

pages/index.js

import React from 'react'
import dynamic from 'next/dynamic';

// Dynamically imported component
const Component = dynamic(import('../components/Component'));
const ForwardedRefComponent = React.forwardRef((props, ref) => (
  <Component {...props} forwardedRef={ref} />
))

export default class Index extends React.Component {
  constructor(props) {
    super(props)
    this.myRef = React.createRef();
  }

  componentDidMount() {
    console.log('this.myRef', this.myRef)
  }

  render() {
    return <ForwardedRefComponent ref={this.myRef}/>;
  }
}

components/Component.js

export default ({ forwardedRef }) => (
  <div ref={forwardedRef}>
    Hi from Component
  </div>
)

Please note: You can also omit the forwardRef api and pass the ref as forwardedRef in directly.

Example

Issues adding this functionality to next.js

I don't think it makes sense to implement it in next now since:

  • react-loadable needs to implement it first
  • React <16.3 needs to be deprecated first
  • won't work with other rendering engines (e.g. preact)

how can i do with the custom Component not wroten by myself ? I cant set

export default ({ forwardedRef }) => (
  <div **ref={forwardedRef}**>
     Hi from Component
  </div>
)
matiastucci commented 4 years ago

I'm trying to use forwardRef on an external component but ref.current is not the actual ref. Not sure if I'm missing something. Can anyone take a look, please?

Here it's my code: https://codesandbox.io/s/objective-benz-qh4ec?file=/pages/index.js:63-73

alvelig commented 3 years ago

If you can't modify an imported component, then you'll have to hack into the DOM.

My workaround (or maybe it's just a forgotten canonical way?) is the following:

const Component = () => {
  const ref = useRef();
  const id = useState(shortid());
  const DynamicComponent = eval('div') // Put whatever variable that contains your dynamic component
  useEffect(() => {
    ref.current = document.getElementById(id);
  }, [DynamicComponent]);
  return <DynamicComponent id={id} />
}

Be aware of that if you change your dynamic component due to some conditions, you need to add those conditions to useEffect so the ref is updated with your component.

matiastucci commented 3 years ago

@alvelig thanks for the response!

Sadly, I'm not sure if I follow this part:

eval('div') // Put whatever variable that contains your dynamic component

Can you explain it more, please?

Thanks 🙏

fcFn commented 3 years ago

@matiastucci You have to wrap your component in a custom component and pass the ref through it.

See your modified example: https://codesandbox.io/s/relaxed-pine-b8f5z.

Note that you can use the custom prop way without using forwardRef (example: https://codesandbox.io/s/busy-jennings-cxll6).

giacomoalonzi commented 3 years ago

@matiastucci You have to wrap your component in a custom component and pass the ref through it.

See your modified example: https://codesandbox.io/s/relaxed-pine-b8f5z.

Note that you can use the custom prop way without using forwardRef (example: https://codesandbox.io/s/busy-jennings-cxll6).

In your example the ref is null, is this the proper result?

fcFn commented 3 years ago

@giacomoalonzi If you check the console pane (not the terminal pane), you'll see that the ref is not null after the editor renders.

igormartimiano commented 3 years ago

@fcFn Much appreciated, thanks!

ical10 commented 3 years ago

Solution

@juancampa You can use the forwardRef api to implement it yourself.

pages/index.js

import React from 'react'
import dynamic from 'next/dynamic';

// Dynamically imported component
const Component = dynamic(import('../components/Component'));
const ForwardedRefComponent = React.forwardRef((props, ref) => (
  <Component {...props} forwardedRef={ref} />
))

export default class Index extends React.Component {
  constructor(props) {
    super(props)
    this.myRef = React.createRef();
  }

  componentDidMount() {
    console.log('this.myRef', this.myRef)
  }

  render() {
    return <ForwardedRefComponent ref={this.myRef}/>;
  }
}

components/Component.js

export default ({ forwardedRef }) => (
  <div ref={forwardedRef}>
    Hi from Component
  </div>
)

Please note: You can also omit the forwardRef api and pass the ref as forwardedRef in directly.

Example

Issues adding this functionality to next.js

I don't think it makes sense to implement it in next now since:

  • react-loadable needs to implement it first
  • React <16.3 needs to be deprecated first
  • won't work with other rendering engines (e.g. preact)

Thanks a bunch! Works with my next.js + typescript project, you just need to add simple snippets below in Component.ts:

interface ComponentProps {
  forwardedRef: React.ForwardedRef<any>;
}

const Component: React.FC<ComponentProps> = ({ forwardedRef }) => {
...
}
balazsorban44 commented 2 years ago

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.