feat: base addition for context handling

[barelyhuman]

pmndrs/valtio#204 (comment)

Note: Needs additional test cases and checks , this is just a base commit for the feature, not the completed version.

[codesandbox-ci]

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 baf1af6:

Sandbox	Source
Vanilla	Configuration
React	Configuration

[dai-shi]

some ideas for the complete version.

// invalid
const state = proxy({
  count: 0,
  inc() {
    ++this.count
  },
})

// valid
const state = proxy({
  count: 0,
  inc() {
    ++state.count
  },
})

// valid
const state = proxy({
  count: 0,
  inc: function () {
    ++state.count
  },
})

// valid
const state = proxy({
  count: 0,
  inc: () => {
    ++state.count
  },
})

// invalid
const state = proxy({
  count: 0,
  inc: function () {
    ++this.count
  },
})

// invalid (technically possible though)
const state = proxy({
  count: 0,
  inc: () => {
    ++this.count
  },
})

// valid
const state = proxy({
  arr: [],
  inc: () => {
    state.arr = [1, 2, 3].map(function(x) { return x / this }, 10)
  },
})

// invalid
const state = proxy({
  count: [],
  inc() {
    (() => { ++this.count })()
  },
})

Basically, if this is pointing to state or developers mean it, we detect it as invalid. Otherwise, valid.

[dai-shi]

On second thought, we don't need to take care of those edge cases. We can just warn this usage in proxy regardless of the usage. After all, it's just a warning for beginners.

[barelyhuman]

we don't need to take care of those edge cases

don't plan to either, but these are just for the tests so that we can be assured that beginners are warned that something is going to be wrong aka, more for our confirmation than for the users

Also,
Avoid using 'this' in valtio.proxy context. It might lead to unexpected results
Is this okay for a message? I think we'd need a little more clarity added to the message, let me know

[dai-shi]

If possible, we want to detect this, but maybe it's hard and not possible across modules?

// invalid
const initialObj = {
  count: 0,
  inc() {
    ++this.count
  },
}

const state = proxy(initialObj)

[dai-shi]

Avoid using 'this' in valtio.proxy context. It might lead to unexpected results
Is this okay for a message? I think we'd need a little more clarity added to the message, let me know

I think it's good. To add a note, we can make sure that the rule is optional.
Something like "using this is valid, but often a pitfall for beginners"

[barelyhuman]

If possible, we want to detect this, but maybe it's hard and not possible across modules?

// invalid
const initialObj = {
  count: 0,
  inc() {
    ++this.count
  },
}

const state = proxy(initialObj)

Hmm, I'll have to add in a few more handlers to see how I can handle that, it should be possible but then I guess the eslint error would show on the proxy(initialObj) line, which can be confusing, I'll try to replicate that

To add a note, we can make sure that the rule is optional.

They can turn it off from the rules , and yes i'll add "using this is valid, but often a pitfall for beginners" to the message

Though, I think people turning the rule off will probably know what it's going to do so makes sense to keep it in the recommended rules

[barelyhuman]

@dai-shi I have a version that detects this

// invalid
const initialObj = {
  count: 0,
  inc() {
    ++this.count
  },
}

const state = proxy(initialObj)

though it's the hackiest solution I've ever written, so I'm testing it with other common setups so it doesn't collide with them.
I'll let you know either way.

[barelyhuman]

@dai-shi want me to set it as ready for review or is there another case we need to check, let me know

[dai-shi]

If you don't have any concerns, it means ready!

[barelyhuman]

Where the warning will be shown in both cases.

[dai-shi]

LGTM. Nice work.

[Aslemammad]

That's a clean & valid code! Thank you for your efforts.