gaearon / react-document-title

Declarative, nested, stateful, isomorphic document.title for React
MIT License
1.86k stars 105 forks source link

Warning: componentWillMount has been renamed — SideEffect(DocumentTitle) #62

Open a-x- opened 5 years ago

a-x- commented 5 years ago

react 16.9.0+ compat

flyingcrp commented 5 years ago

got the same warning. image

       "react": "^16.9.0",
        "react-document-title": "^2.0.3",
        "react-dom": "^16.9.0",
        "react-loadable": "^5.5.0",
        "react-router-dom": "^5.0.1",
a-x- commented 5 years ago

We had not merged the pull request, because of problems in react-document-title

there is not lite way to replace componentWillMount → componentDidMount here.

more info: https://github.com/gaearon/react-side-effect/issues/40#issuecomment-367186386

kud commented 5 years ago

Interested by this too.

mmakarin commented 5 years ago

Just use hooks.

function useDocumentTitle(title) {
  const originalTitle = document.title;

  useEffect(
    () => {
      document.title = title;

      return () => {
        document.title = originalTitle;
      };
    },
    [title, originalTitle]
  );
}
flyingcrp commented 5 years ago

need upgrade this package?

Hypnosphi commented 5 years ago

@mmakarin this will not actually reset the title, because originalTitle updates on the next render. Something like this should work though:

function useDocumentTitle(title) {
  useEffect(
    () => {
      const originalTitle = document.title;
      document.title = title;

      return () => {
        document.title = originalTitle;
      };
    },
    [title]
  );
}
erindepew commented 5 years ago

So your solution works @Hypnosphi solution is on my branch (ignore the tests, they're broken for the reason following) https://github.com/erindepew/react-document-title/tree/convert-to-hooks the only problem is that it seems to also require a change for withSideEffect https://github.com/gaearon/react-side-effect in order to get the .rewind() and .peek(), and I'm not sure there's a way to do that without componentWillMount. Thoughts?

SeanRoberts commented 5 years ago

One issue with the hooks approach is that nested components don't take precedence over their parents: https://codesandbox.io/s/interesting-bartik-stwys (see useDocumentTitle.ts and useDocumentTitle.test.tsx)

Given that children are mounted before parents I'm not really sure what the best approach to solving this would be. Especially tricky when you consider sibling components as well and determining which should take precedence

Edit: I think I might just be re-stating what @erindepew said above

erindepew commented 5 years ago

Great point, completely forgot about that as well. So seems like refactoring to hooks isn't really an option for this component.

SeanRoberts commented 5 years ago

Yeah I think essentially the heavy lifting here is handled by react-side-effect, and react-side-effect depends on componentWillMount. The issue for that is here: https://github.com/gaearon/react-side-effect/issues/54

Dan stated in that issue that react-side-effect doesn't really make sense to him anymore and that a more modern approach might be to use context. It looks like react-helmet-async is doing just that, so maybe that's the way to go.

machineghost commented 4 years ago

What's needed to make this happen? Does a solution need to be picked by the maintainers (it seems like context is the way to go?) Is a PR needed?

The last comment here is from 2019; how can we make this happen in 2020?

RomanKontsevoi commented 4 years ago

Please, fix this issue

kud commented 4 years ago

I think you should use https://github.com/staylor/react-helmet-async instead.

  <HelmetProvider>
    <App>
      <Helmet>
        <title>Hello World</title>
      </Helmet>

      <h1>Hello World</h1>
    </App>
  </HelmetProvider>
peterboyer commented 4 years ago

Seeing as the last release is back in 12 Apr 2017, I just had to solve this for myself and ended up using react-helmet for a drop-in replacement of DocumentTitle.

// custom-document-title.tsx

import React from "react";
import Helmet from "react-helmet";

export type IDocumentTitleProps = {
  title?: string;
  children?: React.ReactElement;
};

export function DocumentTitle(props: IDocumentTitleProps) {
  const { title, children } = props;
  return (
    <>
      <Helmet>
        <title>{title}</title>
      </Helmet>
      {children}
    </>
  );
}

export default DocumentTitle;

Usage, exactly the same as react-document-title and nested declarations work just as you'd expect. Drop-in replacement for this package.

import DocumentTitle from "./custom-document-title"

<DocumentTitle title="My Title">
shiznit013 commented 3 years ago

@ptboyer I believe your solution will still generate a warning, because react-helmet uses react-side-effect. I had come up with a very similar drop-in replacement using react-helmet-async.

import React from 'react';
import { Helmet } from 'react-helmet-async';

interface Props {
    children?: React.ReactNode;
    title: string;
}

export const DocumentTitle = ({ children, title }: Props) =>
    <>
        <Helmet>
            <title>{title}</title>
        </Helmet>
        {children}
    </>;

Note: We don't use default exports in our project. You'll still need to setup the HelmetProvider as described earlier in the comments

gaelollivier commented 3 years ago

If anyone is looking for a short hook solution that matches this package behavior, this should do it:

import React from 'react';

const DEFAULT_PAGE_TITLE = 'My Website';

const DocumentTitleContext = React.createContext(DEFAULT_PAGE_TITLE);

/**
 * Update the page title with the given prop
 * NOTE: We use a context so that we can restore the title to whichever value is specified by the nearest parent
 */
export const DocumentTitle = ({
  title,
  children,
}: {
  title: string;
  children: React.ReactNode;
}) => {
  const parentTitle = React.useContext(DocumentTitleContext);

  React.useEffect(() => {
    document.title = title;

    return () => {
      document.title = parentTitle;
    };
  }, [title, parentTitle]);

  return <DocumentTitleContext.Provider value={title}>{children}</DocumentTitleContext.Provider>;
};

Previous solutions that snapshot the title in useEffect didn't seem to work for me and were causing flickering in the title. This approach should consistently restore the title to the one defined by the nearest parent.

Sinled commented 3 years ago

@gaelduplessix maybe i am missing something, but with your solution, if you nest several DocumentTitle

     <DocumentTitle title="top title">
        <DocumentTitle title="title">
          <DocumentTitle title="inner title">
            <h1>some content</h1>
          </DocumentTitle>
        </DocumentTitle>
      </DocumentTitle>

final document.title will be top title not the inner title