search

dai-shi / proxy-compare

Compare two objects using accessed properties with Proxy
https://www.npmjs.com/package/proxy-compare
MIT License
283 stars 18 forks source link

feat: Rename affected to accessed. #52

Closed stephenh closed 1 year ago

codesandbox-ci[bot] commented 1 year ago

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit b42ebc3f9e756261b6ff23b310a2592635bd4506:

Sandbox Source
Vanilla Configuration
Vanilla Typescript Configuration
React Configuration
React Typescript Configuration
dai-shi commented 1 year ago

Hm, changing only readme is rather confusing. If we were to change it, we'd want for code too. Even though I agree the change might be better, I'm not 100% sure how much helpful for future developers. So, I want a review from someone else. My concern is it's a WeakMap, and it's already non-intuitive. Unless they understand it fully, changing the name is only half a way.

At the same time, this change isn't very breaking, so if you are willing to help changing all its family, I'm not very against to it.

stephenh commented 1 year ago

Hm, changing only readme is rather confusing. If we were to change it, we'd want for code too.

Ah yeah, totally agreed; FWIW I put this disclaimer in the PR overview, but should have included it in my other comment that linked to this, I just meant for this draft PR to be "see how the change feels in the updated readme" (being kind of lazy :-)) and yep, before flipping the PR out of draft, would make the rename in the code as well.

I'll push up the rest in a few days.

So, I want a review from someone else.

Cool! Happy to have a tie-breaker weigh in. :-D

dai-shi commented 1 year ago

Yeah, note that there's still have 50% possibility that this PR won't be merged.

API docs are generated with yarn run apidoc. Let's call new function, getPathList(accessed: WeakMap)

stephenh commented 1 year ago

Yeah, note that there's still have 50% possibility that this PR won't be merged.

Cool, understood.

API docs are generated with yarn run apidoc.

Oh neat! Updated.

Let's call new function, getPathList(accessed: WeakMap)

Done.

I've made the rest of the updates, and kept affectedToPathList as a deprecated function.