Fixes bug where Astro can't distinguish between two equal components…

[matsdahlin]

Changes

Includes a components props as distinguishing factor when generating hash for astroId, closes #844

Testing

Tested manually with example project. Would like help creating a unit test, but I can't get any Astro tests to run on my computer.

Docs

Bug fix only, no change needed in docs.

[changeset-bot]

🦋 Changeset detected

Latest commit: 2d728a0

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

This PR includes changesets to release 1 package

Name	Type
astro	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]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

astro-www – ./www

🔍 Inspect: https://vercel.com/pikapkg/astro-www/9yMQ68zmMkABAghrDMGGtgVu6UXc
✅ Preview: https://astro-www-git-fork-matsdahlin-bugfix-astroid-for-ed4ff4-pikapkg.vercel.app

astro-docs – ./docs

🔍 Inspect: https://vercel.com/pikapkg/astro-docs/5G6Sf8XqtreF3Y48S4pedS2vvJiR
✅ Preview: https://astro-docs-git-fork-matsdahlin-bugfix-astroid-fo-28552b-pikapkg.vercel.app

[FredKSchott]

what happens if two components with the same props? It sounds like we end up with two astro-root components with the same uid attribute.

Curious if this is something that we should optimize for (just render all matching components with the same hydrate script) or be super explicit about (each component gets a completely unique id, not based on html).

[matthewp]

We should get rid of these inline scripts altogether imo and colocate everything in the head. A small inlined custom element would make these a lot easier.

But I think we should merge this fix in first because this seems like a bad bug to me.

[matsdahlin]

what happens if two components with the same props? It sounds like we end up with two astro-root components with the same uid attribute.

Curious if this is something that we should optimize for (just render all matching components with the same hydrate script) or be super explicit about (each component gets a completely unique id, not based on html).

My opinion is that the way it currently works goes against established patterns of hydration, and it's not the expected behavior.

Using a modified version of the React counter component as an example:

import React, { useState } from 'react';

export default function Counter({ initialValue, children }) {
  const [count, setCount] = useState(initialValue);
  const add = () => setCount((i) => i + 1);
  const subtract = () => setCount((i) => i - 1);

  return (
    <>
      <div className="counter">
        <button onClick={subtract}>-</button>
        <pre>{count}</pre>
        <button onClick={add}>+</button>
      </div>
      <div className="children">{children}</div>
    </>
  );
}

And then use two instances with different initial values in an Astro file:

<Counter initialValue={0} client:idle />
<Counter initialValue={10} client:idle />

The above code would result in both Counter components being hydrated with the same initialValue.

I believe that using the components props as part of the hash generation gives us both the current optimization (identical components with identical props will get the same hydrate script) and generates unique hydrate script when components differ only in what prop values get set from Astro.

[jasikpark]

This looks good to me as a fix even if there are questions about whether the overarching API provided needs reworking!

[matthewp]

Going to merge, let's work to improve how we do hydration in another issue (maybe an RFC for an overhaul).