Skip to content

fix: better isPromise check for proxy objects #11178

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

Merged
merged 1 commit into from
Jun 5, 2024

Conversation

theoephraim
Copy link
Contributor

Changes

Adjusts the isPromise utility to first check for the presence of the 'then' key before accessing it to check the type.

The absence of this check on a Proxy object was causing an error to be thrown - as my Proxy object throws on access of an unknown keys.

Testing

This should be a totally transparent change, and not quite worth setting up a test IMO.
Note that a few tests were failing for me locally, but they are failing on main as well, so seem to be unrelated.

Docs

N/A - This should hopefully be a totally transparent change that affects nothing other than this very specific error scenario.

Copy link

changeset-bot bot commented Jun 3, 2024

🦋 Changeset detected

Latest commit: bc851c9

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

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 Jun 3, 2024
'astro': patch
---

fix: make isPromise work on proxy objects
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
fix: make isPromise work on proxy objects
Fixes a case where a `Promise` on a `Proxy` would not be handled correctly

I'm not sure about the wording "on a" mainly because idk much about proxies. Any suggestions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about

Improves `isPromise` utility to check the presence of `then` on an object before trying to access it - which can cause undesired side-effects on Proxy objects

@theoephraim theoephraim force-pushed the is-promise-proxy-bug branch from b7b1cb3 to 6095471 Compare June 4, 2024 19:51

Verified

This commit was signed with the committer’s verified signature.
zmievsa Stanislav Zmiev
@theoephraim theoephraim force-pushed the is-promise-proxy-bug branch from 6095471 to bc851c9 Compare June 4, 2024 19:58
@bluwy bluwy merged commit 1734c49 into withastro:main Jun 5, 2024
13 checks passed
@astrobot-houston astrobot-houston mentioned this pull request Jun 5, 2024
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.

None yet

3 participants