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

Possible MetaFunction bug #7584

Closed
1 task done
ErlendS opened this issue Oct 3, 2023 · 8 comments · Fixed by #7594
Closed
1 task done

Possible MetaFunction bug #7584

ErlendS opened this issue Oct 3, 2023 · 8 comments · Fixed by #7594
Labels
bug Something isn't working feat:links-meta Issues related to links()/meta()

Comments

@ErlendS
Copy link

ErlendS commented Oct 3, 2023 •
edited
Loading

What version of Remix are you using?

2.0.1

Are all your remix dependencies & dev-dependencies using the same version?

  • Yes

Steps to Reproduce

The problem:

When passing the meta function an array for it to return, and one of the objects in the array has the property: "tagName: 'link'", it causes an hydration error.

Steps to Reproduce

I tried creating a fork with a test to produce the bug: https://github.com/ErlendS/remix-possible-metafunction-bug/blob/main/integration/bug-report-test.ts

But the test passes...

However, when I created (what I believe to be) the exact same setup in stackblitz, the error appears:
https://stackblitz.com/edit/remix-run-remix-rvjcj7?file=app%2Froutes%2F_index.tsx

How to see the error:

  • Open the stackbliz link.
  • Click "Open in New Tab" in the top-right corner
  • Check the console.

It should give out a warning like : Warning: Expected server HTML to contain a matching <meta> in <head>.

If you then go to the 'Network' tab and view the response from the document being returned by Remix, the <head> should contain <link rel="icon" href="https://remix.run/favicon-32.png" type="image/png"/>.

Continue to the 'Elements' tab and check the <head> in the document and you will see it is incorrectly rendering: <meta rel="icon" href="https://remix.run/favicon-32.png" type="image/png">

The server and client have mismatching tags.

Additional info:

I hope you don't mind, but since its not much code I thought I could add the code here to make it easier to view :

import { type MetaFunction, json } from "@remix-run/node";

export const meta: MetaFunction<typeof loader> = (loader) => {
  // ❌ this causes hydration error.
  const metaArray = loader.data?.metaArray
  if (metaArray) {
    return metaArray;
  }
  
  // The problem looks to be the objects in the array being mutated,
  // if we copy the objects in the array it works.
  // const copy = metaArray?.map((obj) => Object.assign({}, obj));
  // return copy

  return []
};

export async function loader() {

  // contrived example, but say you're on a $slug route and you need to do some checks to get the 'href' for the favIcon.
  const metaArray = [
    {
      tagName: 'link',
      rel: 'icon',
      href: 'https://remix.run/favicon-32.png',
      type: 'image/png',
    },
  ];

  return json({ metaArray });
}

export default function Index() {
  return (
    <div>
      <h1>Welcome to Remix</h1>
    </div>
  );
}

Looking a bit in the Remix code, I think this might be the issue:

remix/packages/remix-react/components.tsx

Line 516 in a20ae7f

delete metaProps.tagName;

it could be when "tagName" is deleted, the reference object is mutated, therefore when the meta function runs again on the client it assigns the object as a meta tag.

And if this is the cause, it could possibly be fixed a few lines above: https://github.com/remix-run/remix/blob/a20ae7fb0727212ac52bdc687513c61851ac4014/packages/remix-react/components.tsx#L509C25-L509C34
by cloning the object so it doesn't point to the same object in memory.

Expected Behavior

When the meta function receives the array it should return as a prop, and one of the objects in the array has the property: "tagName: 'link'", we should not get a hydration error. It should render a <link> tag on the server and on the client.

Actual Behavior

The Meta component returns a <link> tag on the server and <meta> on the client.
@sergiodxa
Copy link
Member

Are you using React canary?

@ErlendS
Copy link
Author

ErlendS commented Oct 4, 2023

No. The stackblitz example is using react 18.2.0

@brophdawg11 brophdawg11 added the feat:links-meta Issues related to links()/meta() label Oct 4, 2023
@brophdawg11
Copy link
Contributor

I think you're correct in that the mutation is causing the issue, so subsequent renders don't pick up on tagName. I think we can just change it to just destructure off the tagName separate from the rest of the props:

if ("tagName" in metaProps) {
  let { tagName, ...rest } = metaProps;
  if (!isValidMetaTag(tagName)) {
    console.warn(
      `A meta object uses an invalid tagName: ${tagName}. Expected either 'link' or 'meta'`
    );
    return null;
  }
  let Comp = tagName;
  return <Comp key={JSON.stringify(rest)} {...rest} />;
}

I'll try to get a fix in for this soon 👍

@brophdawg11 brophdawg11 self-assigned this Oct 4, 2023
@ErlendS
Copy link
Author

ErlendS commented Oct 4, 2023

That's great to hear!

If you have have time, I am also curious if it was anything wrong with my test file: https://github.com/ErlendS/remix-possible-metafunction-bug/blob/main/integration/bug-report-test.ts. It should be the exact same example as the one shared in Stackblitz, but it passes the test.

@charklewis
Copy link

Thanks for looking into this @brophdawg11 and @ErlendS, I am having this issue too.

@brophdawg11 brophdawg11 mentioned this issue Oct 5, 2023
Merged
@brophdawg11 brophdawg11 linked a pull request Oct 5, 2023 that will close this issue
Avoid mutating meta object when tagName is specified #7594
Merged
@brophdawg11 brophdawg11 added bug Something isn't working awaiting release This issue has been fixed and will be released soon and removed bug:unverified labels Oct 5, 2023
@brophdawg11 brophdawg11 removed their assignment Oct 5, 2023
@brophdawg11
Copy link
Contributor

This is resolved by #7594 and will be included in the next release 👍

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 2.1.0-pre.0 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 2.1.0 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@brophdawg11 brophdawg11 removed the awaiting release This issue has been fixed and will be released soon label Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working feat:links-meta Issues related to links()/meta()
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Avoid mutating meta object when tagName is specified remix-run/remix
4 participants
@sergiodxa @brophdawg11 @charklewis @ErlendS