Skip to content

feature/proxyMap-new-methods#1225

Draft
overthemike wants to merge 3 commits into
mainfrom
feature/proxyMap-new-methods
Draft

feature/proxyMap-new-methods#1225
overthemike wants to merge 3 commits into
mainfrom
feature/proxyMap-new-methods

Conversation

@overthemike
Copy link
Copy Markdown
Collaborator

Related Bug Reports or Discussions

#1224
Fixes #
n/a

Summary

Added getOrInsert and getOrInsertComputed methods on proxyMap

Check List

  • [ x ] pnpm run fix for formatting and linting code and docs

@vercel
Copy link
Copy Markdown

vercel Bot commented May 10, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
valtio Error Error May 11, 2026 1:15am

Request Review

@codesandbox-ci
Copy link
Copy Markdown

codesandbox-ci Bot commented May 10, 2026

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.

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 10, 2026

More templates

npm i https://pkg.pr.new/valtio@1225

commit: b0b71b7

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 10, 2026

Size Change: +83 B (+0.54%)

Total Size: 15.4 kB

Filename Size Change
./dist/esm/vanilla/utils.mjs 3.96 kB +40 B (+1.02%)
./dist/vanilla/utils.js 4.01 kB +43 B (+1.08%)
ℹ️ View Unchanged
Filename Size
./dist/esm/index.mjs 63 B
./dist/esm/react.mjs 694 B
./dist/esm/react/utils.mjs 271 B
./dist/esm/utils.mjs 68 B
./dist/esm/vanilla.mjs 2.42 kB
./dist/index.js 243 B
./dist/react.js 686 B
./dist/react/utils.js 292 B
./dist/utils.js 244 B
./dist/vanilla.js 2.46 kB

compressed-size-action

@overthemike
Copy link
Copy Markdown
Collaborator Author

@dai-shi I still need to write tests for this but I wanted to get your initial opinion on it before I do. Especially the type updates. Needed to put those in order for ts not to yell but it feels somewhat fragile. Necessary, but fragile. Thoughts?

@dai-shi
Copy link
Copy Markdown
Member

dai-shi commented May 10, 2026

Needed to put those in order for ts not to yell but it feels somewhat fragile. Necessary, but fragile. Thoughts?

Should be fine as long as our TS tests work with [TS-ONLY-... hack.
I'm not sure why it's necessary though.

@overthemike
Copy link
Copy Markdown
Collaborator Author

overthemike commented May 11, 2026

I'm not sure why it's necessary though.

Typescript yells when trying to run the tests using pnpm test without it.
error TS2353: Object literal may only specify known properties, and 'getOrInsert' does not exist in type...

@dai-shi
Copy link
Copy Markdown
Member

dai-shi commented May 11, 2026

lemme try

@overthemike
Copy link
Copy Markdown
Collaborator Author

overthemike commented May 11, 2026

b0b71b7 was originally what I wanted to push up, but the problem is still persisting for me. When I run pnpm test locally, this is what I'm getting:

. test:format$ prettier . --list-different
└─ Running...
. test:types$ tsc --noEmit
│ src/vanilla/utils/proxyMap.ts(182,5): error TS2353: Object literal may only specify known properties, and 'getOrInsert' does not exist in typ…
└─ Failed in 2.8s at /Users/mike/Projects/valtio   #  <==  \(><)/ 
. test:lint$ eslint .
└─ Running...
. test:spec$ vitest run
[15 lines collapsed]
│  ✓ |valtio| tests/subscribe.test.tsx (14 tests) 17ms
│  ✓ |valtio| tests/watch.test.tsx (8 tests) 12ms
│  ✓ |valtio| tests/deepClone.test.tsx (5 tests) 5ms
│  ✓ |valtio| tests/performance.test.tsx (1 test) 128ms
│  ✓ |valtio| tests/mapset.test.tsx (2 tests) 24ms
│  ✓ |valtio| tests/ref.test.tsx (4 tests) 43ms
│  Test Files  18 passed (18)
│       Tests  285 passed | 1 skipped (286)
│    Start at  19:00:00
│    Duration  2.49s (transform 613ms, setup 970ms, collect 3.07s, tests 2.61s, environment 10.74s, prepare 1.37s)
└─ Running...
 ELIFECYCLE  Command failed with exit code 2.
 ELIFECYCLE  Test failed. See above for more details.

@dai-shi
Copy link
Copy Markdown
Member

dai-shi commented May 11, 2026

that doesn't reproduce locally on my end (nor on CI as you see).

@overthemike
Copy link
Copy Markdown
Collaborator Author

that doesn't reproduce locally on my end (nor on CI as you see).

Good to know and glad I asked. I'll debug and figure it out and then get started with the tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants