graphql-tools icon indicating copy to clipboard operation
graphql-tools copied to clipboard

Copy Symbols as well in `copyOwnPropsIfNotPresent`

Open Hetch3t opened this issue 3 years ago • 6 comments
trafficstars

Hi!

I had issues with mocking lib - we use ObjectId from Mongo as an ID scalar type, so after introducing mocking to our schema it started to fail. After hefty amount of time debugging it turned out that ObjectId is implemented in such a way that is is stored as buffer as Symbol(id). Function that is being used in @graphql-tools/mock uses Object.getOwnPropertyNames which doesn't list Symbols. The solution I've implemented you can see in this PR.

Hetch3t avatar Jun 04 '22 18:06 Hetch3t

⚠️ No Changeset found

Latest commit: 5539ec0de06de662f682df8e0041bbb47813a75f

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.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

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

changeset-bot[bot] avatar Jun 04 '22 18:06 changeset-bot[bot]

@Hetch3t is attempting to deploy a commit to the The Guild Team on Vercel.

A member of the Team first needs to authorize it.

vercel[bot] avatar Jun 04 '22 18:06 vercel[bot]

Thanks for the PR! Would you add some unit tests to prevent future regressions and changeset(using yarn changeset)?

ardatan avatar Jun 05 '22 14:06 ardatan

We just encountered this same issue when attempting to mock the MongoID scalar type (started getting error "Cannot create Buffer from undefined"), which led me to this PR. I quick/dirty tested the proposed changes here and they seem to work wonderfully🥇!

Other than needing tests, is there anything else that is blocking this fix from making it into a release? At over a year old, is this PR still valid? I see that it's listed in the project roadmap, but just wanted to give it a bump.

bkalbs-wbd avatar Nov 28 '23 20:11 bkalbs-wbd

@bkalbs-wm would you be able to contribute a test for this?

Urigo avatar Dec 02 '23 16:12 Urigo

@Urigo i can give it a go once I get some breathing room from this work deadline i am on. would like to help however i can. I can open up a new PR once ready.

bkalbs-wbd avatar Dec 06 '23 15:12 bkalbs-wbd