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

Supports Uint8Array/Uint16Array/Uint32Array for serialize props #4669

Merged
merged 17 commits into from
Sep 21, 2022

Conversation

aggre
Copy link
Contributor

@aggre aggre commented Sep 8, 2022

Changes

  • Allow astro-island to handle Uint8Array/Uint16Array/Uint32Array correctly
  • Currently, for example, new Uint8Array([1,2,3]) is handled as {0: 1, 1: 2, 2: 3} and loses the type

Testing

I couldn't find any hydration tests for astro-island.props. If there is, I will gladly update the test.

Docs

This changes doesn't affect the docs.

@changeset-bot
Copy link

changeset-bot bot commented Sep 8, 2022

🦋 Changeset detected

Latest commit: 84bb3a5

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

This PR includes changesets to release 16 packages
Name Type
astro Minor
@e2e/astro-component Patch
@e2e/error-cyclic Patch
@e2e/error-react-spectrum Patch
@e2e/error-sass Patch
@e2e/errors Patch
@e2e/hydration-race Patch
@e2e/lit-component Patch
@e2e/preact-component Patch
@e2e/react-component Patch
@e2e/solid-component Patch
@e2e/solid-recurse Patch
@e2e/svelte-component Patch
@e2e/e2e-tailwindcss Patch
@e2e/ts-resolution Patch
@e2e/third-party-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

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Sep 8, 2022
@aggre
Copy link
Contributor Author

aggre commented Sep 8, 2022

pnpm run format --list and pnpm run build fail on the CI, but in my local environment they finish correctly. What's causing this?

@matthewp
Copy link
Contributor

matthewp commented Sep 8, 2022

Change looks good! I think there's something wrong with our deps as other PRs are failing. Will update this branch once that's fixed.

@natemoo-re
Copy link
Member

I just merged some unit tests for prop serialization, please add these to the test in packages/astro/test/serialize.test.js!

@aggre
Copy link
Contributor Author

aggre commented Sep 9, 2022

@matthewp @natemoo-re Thank you for the merge commit and creating tests!

I've written additional tests and passed CI, so I think the changes are ready to review again.

@matthewp
Copy link
Contributor

matthewp commented Sep 9, 2022

Looks great, thank you!

@@ -0,0 +1,5 @@
---
'astro': patch
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be minor as this is technically a new feature, the ability to pass through typed arrays.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, it makes sense!

@matthewp matthewp added the semver: minor Change triggers a `minor` release label Sep 9, 2022
@github-actions github-actions bot added the pkg: create-astro Related to the `create-astro` package (scope) label Sep 11, 2022
@github-actions github-actions bot removed the pkg: create-astro Related to the `create-astro` package (scope) label Sep 13, 2022
Copy link
Member

@natemoo-re natemoo-re left a comment

Choose a reason for hiding this comment

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

Looks great, thank you! This will go out when we're ready for our next minor!

@matthewp matthewp merged commit a961aa3 into withastro:main Sep 21, 2022
@astrobot-houston astrobot-houston mentioned this pull request Sep 21, 2022
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) semver: minor Change triggers a `minor` release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants