Skip to content
This repository has been archived by the owner on Dec 31, 2020. It is now read-only.

displayName is set after observer, observer could use a proxy to fetch it #44

Closed
natew opened this issue Jan 13, 2019 · 5 comments
Closed
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@natew
Copy link

natew commented Jan 13, 2019

Since we write it as:

const Item = observer(() => <div />)
Item.displayName = 'Item'

It won't pick up the displayName. Perhaps observer could use a proxy to pick that up:

function observer(a) {
  return new Proxy(a, { set() { /* pick up displayName and apply it to reaction here */ } })
}

Not sure if Mobx accepts it "after its created" so to speak, but for now I just see a ton of observer() in my reaction logs which isn't very helpful.

@danielkcz
Copy link
Collaborator

danielkcz commented Jan 13, 2019

Yea, I noticed that as well and it's a kinda main reason why I am mostly using useObserver whenever I can. In other cases, I just have a named function.

export default observer(function Item() {
  return <div />
})

Either way, these are just workarounds for sure. Personally, I haven't use Proxy for anything yet. There is an obvious problem it's not supported in IE11 so it would need some ponyfill. And it shouldn't be used in a production code.

Um, could you try to make some preliminary PR? Ideally with tests. I am definitely preoccupied for next weeks.

@danielkcz danielkcz added bug Something isn't working help wanted Extra attention is needed labels Jan 13, 2019
@xaviergonz
Copy link
Contributor

There's always defineProperty with a getter and setter, which is like the poor's man proxy

@xaviergonz
Copy link
Contributor

xaviergonz commented Feb 6, 2019

Either way though, since Reaction takes a plain string any name change after the fact would require a reaction destruction and re-creation, so maybe it could be added as an option to observer instead (displayName?: string)?

const Item = observer(() => <div />, { displayName: "Item" })

@danielkcz
Copy link
Collaborator

That's a really ugly escape hatch :) I would rather avoid things that cannot be compiled away from a production build. I cannot help it but using named function instead of anonymous one comes to be as the best solution really.

@danielkcz
Copy link
Collaborator

Another approach I just spotted in some chatroom. This way the observer grabs the name correctly.

const Item = () => <div />

export default observer(Item)

I've decided to reconsider this whole thing. It's not really a bug. It's coming from the way JS works. Some sufficient approaches have been described, for now, so I am closing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants