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

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

Merged
merged 2 commits into from
Jul 27, 2021
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
15d3acb
Fixes bug where Astro can't distinguish between two equal components…
matsdahlin Jul 24, 2021
2d728a0
Merge branch 'main' into bugfix/astroid-for-hydration-script-ignores-…
matsdahlin Jul 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/chilly-gorillas-wash.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

This includes the props passed to a hydration component when generating the hash/id. This prevents multiple instances of the same component with differing props to be treated as the same component when hydrated by Astro.
3 changes: 2 additions & 1 deletion packages/astro/src/internal/__astro_component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,8 @@ export function __astro_component(Component: any, metadata: AstroComponentMetada
}

// If we ARE hydrating this component, let's generate the hydration script
const astroId = hash.unique(html);
const stringifiedProps = JSON.stringify(props);
const astroId = hash.unique(html + stringifiedProps);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@matsdahlin matsdahlin Jul 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

const script = await generateHydrateScript({ instance, astroId, props }, metadata as Required<AstroComponentMetadata>);
const astroRoot = `<astro-root uid="${astroId}">${html}</astro-root>`;
return [astroRoot, script].join('\n');
Expand Down