t4t5 / nostr-react

React Hooks for Nostr ðŸĶĪ
MIT License
86 stars 14 forks source link

how to update active relay list #8

Closed wds4 closed 1 year ago

wds4 commented 1 year ago

I was wondering what the best way would be to update the list of active relays; for example, if the user wants to change the relay list in settings. It's possible that I am just not sufficiently adept at react to figure this out so I apologize in advance if there is a simple solution and I am not seeing it!

I will post code to show some of what I've tried (unsuccessfully) in the next message in this thread.

wds4 commented 1 year ago

Here is sample code from my app. My goal here is to load the user's list of preferred relays from sqlite3, which of course is an async operation. I am using this.state.relayUrls to store the list of preferred relays, which starts as an empty array and then gets populated with fetchRelays and setState. I've verified that fetchRelays is returning the correct list. My expectation is that NostrProvider should rerender once that happens. However, it appears that the relay list never gets updated. The only way I can get this to work is if I populate relayUrls from the start.

I think that if I figure out how to do this right, I'll probably be able to figure out the above scenario where the user updates the list of preferred relays and I want it to update on the fly.

Can you help me see what I am missing? (Something simple I hope!)

// my goal is to avoid hardcoding the list like this 
const relayUrls = [
    "wss://nostr-pub.wellorder.net",
    "wss://nostr-relay.untethr.me",
    "wss://relay.damus.io",
    "wss://nostr-relay.wlvs.space",
    "wss://nostr.fmt.wiz.biz",
    "wss://nostr.oxtr.dev",
];

export default class App extends React.Component {
    constructor(props) {
        super(props);
        this.state = {
            relayUrls: [], // init as an empty array 
           //  relayUrls: relayUrls, // the main feed only seems to work if I populate relayUrls from the outset
        }
    }

    async componentDidMount() {
        var relayUrls = await fetchRelays("active") // fetch from sqlite3
        this.setState({relayUrls:relayUrls})
        // this.forceUpdate(); // this doesn't fix the problem either 
    }
    render() {
        return (
            <NostrProvider relayUrls={this.state.relayUrls} debug>
                <fieldset id="app" >
                    <pre>
                       (this element successfully rerenders as I would expect)
                        {typeof this.state.relayUrls}
                        {this.state.relayUrls.length}
                        {this.state.relayUrls}
                    </pre>
                    <Router>
                        <Routes>
                            <Route path="/" element={<LandingPage />} />
                            <Route path="/MainFeed" element={<MainFeed />} />
                            <Route path="/UserProfile" element={<UserProfile />} />
                            // etc for other pages
                        </Routes>
                    </Router>
                </fieldset>
            </NostrProvider>
        );
    }
}
t4t5 commented 1 year ago

Hi, thanks for sharing your code!

Hmm, right now I believe there's no simple way to update the relay list without refreshing the page. It's definitely something we should add though! I'm thinking something like this:

const { updateRelayList, connectedRelays } = useNostr() // "updateRelayList" doesn't exist yet

const addRelayer = (url) => {
  updateRelayList([
     ...connectedRelays,
     url,
  ])
}

If anyone can implement this, PRs are very welcome! 🙏

wds4 commented 1 year ago

I agree - something like updateRelayList would definitely be useful. That would be straightforward to use whenever the user decides to modify the list of active relays.

As for dropped relays, I suppose one could use setInterval() to call updateRelayList periodically. Although that seems a bit inelegant, so perhaps set up a listener on connectedRelays so that whenever one gets dropped, it would call updateRelayList which would reconnect whatever's been dropped.

I'll see if I can get a PR put together. Although I don't know how long it might take me, so don't let me stop anyone else who's interested from working on it too!

wds4 commented 1 year ago

In addition to updateRelayList (above), what would you think of triggering an automatic reconnect after a relay disconnects? Something like this:

<NostrProvider relayUrls={relayUrls} debug={true} autoReconnect={true} >
  <App />
</NostrProvider>

I've created a branch for basic autoReconnect functionality, with the default behavior being not to auto-reconnect at all, that seems to work in my client, although I haven't yet made a pull request because I'm wondering whether I should move the logic (currently, simply: if (autoReconnect)) for whether and when to make a reconnection attempt from where is is now (in the relay.on("disconnect"... function block) to OnDisconnectFunc. (I assume this is why that function was created in the first place, for stuff like this? Let me know if I misunderstand 😄 ) . Ultimately, this logic needs to be more complicated, and different clients will likely want to make different choices. Should we give up after N failed attempts? Should we wait a certain amount of time T before each reconnect attempt? If yes, then should N and T be optional parameters passed to NostrProvider, maybe specific to each relay, or maybe the same for all relays?

Overall, I like the nostr philosophy (as I understand it) of keeping the base protocol as simple as possible, allowing as much flexibility as possible for higher "layers." Following that philosophy, perhaps the client should handle everything via a simple updateRelayList function (as discussed above) with no need for autoReconnect. But to do that, some devs may want to keep track of things like which relays are reliable and which are not. So how would that work? Not really clear to me. Maybe:

const { numDisconnects, updateRelayList , connectedRelays } = useNostr() // neither "numDisconnects" nor "updateRelayList" exists yet

My thinking right now is to include the simple autoReconnect functionality (which I could push right now, if desired) for those devs who might find it useful, and then solicit opinions whether to make autoReconnect more feature rich, leave it simple, or scrap it entirely once updateRelayList works.

wds4 commented 1 year ago

I figured it would be easier to discuss this if I went ahead with the pull request even if it likely still needs some editing 😄