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.

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

Add a propType for receiving a raw value directly #13046

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

effulgentsia
Copy link

@effulgentsia effulgentsia commented Jan 22, 2025

Changes

  • Adds a propType for using a parsed-from-JSON value directly, without reviving sub-objects via tuples. This allows web pages generated by external systems (not Astro) to use Astro islands without needing to encode props containing nested but basic (representable with JSON) data into tuples.

Testing

  • No tests added yet. Wanting to get confirmation on whether Astro maintainers are open to this PR before I spend time on adding tests.

Docs

  • No docs added yet. Wanting to get confirmation on whether Astro maintainers are open to this PR before I spend time on adding docs.

Copy link

changeset-bot bot commented Jan 22, 2025

⚠️ No Changeset found

Latest commit: 12c5a30

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Jan 22, 2025
@ascorbic
Copy link
Contributor

Could you give a little more info on how this would be used?

Copy link

codspeed-hq bot commented Jan 22, 2025

CodSpeed Performance Report

Merging #13046 will not alter performance

Comparing effulgentsia:main (12c5a30) with main (f576519)

Summary

✅ 6 untouched benchmarks

@effulgentsia
Copy link
Author

I'm one of the maintainers of Drupal. I'm working with a team to add a visual page builder to Drupal. Users would be able to drag and drop components and there'd be a UI for editing the props, filling in either static values or sourcing the values from other content that's in Drupal. One of the component types we want to support is Astro islands.

Drupal uses PHP to server-render pages. So from PHP we'd be generating HTML like...

<astro-island ... props="$serialized_props">
  <template data-astro-template="slot1">$slot1</template>
  <template data-astro-template="slot2">$slot2</template>
  ...
</astro-island>

If I have $props as a PHP array (PHP arrays can be keyed with either numbers or strings, and get serialized to json as either arrays or objects depending on that) with depth (e.g., I might have $props['a']['b'][0]['c']), I'd like to be able to do:

$serialized_props = json_encode(array_map((prop) => [12, prop], $props));

rather than:

$serialized_props = json_encode(convert_every_item_at_every_depth_to_a_tuple($props));

It's not the end of the world if we need to do the latter, but I thought maybe other people/projects might have a similar use case and not want to write their own custom serializer.

@ascorbic
Copy link
Contributor

Interesting proposal! I don't see an issue with this specific proposal, but I am a bit more concerned with something that is implicitly making the serialisation format a public API. Right now, while we have no specific plans to change the format, we can change it at will without worrying about breaking any other implementation. If you're manually constructing islands, there's a risk that it would break in a patch release. Realistically you'd need to be responsible for checking yourself that there are no breaking changes in the format in new releases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants