Add Path #6
Add Path #6
Conversation
|
Thanks Simon, I removed all the auto-generated files from main and from your branch, if you can pull main into yours to get this updated that would be great. Also rather than just passing path, I think it would be good to pass other io client options to the function if that's clear? |
|
Yeah, you mean, for example, the reconnection parameter and others as well. Yeah, I started that, but for me, it was more important to get the path going :D I can make a new pull after this one and add the others. |
|
It looks like the auto-generated files are removed. You pushed it already to mean. I see the commit in this PR as well. |
|
What I meant is rather than a |
|
Ah, I got it. I will change that. It makes it easy to add different options in the future. |
|
Please check the last commit. I added some variables already to provide some more options. Is that the way you intended it to be? |
| const defaultOptions: SocketOptions = { | ||
| path: '/socket.io', | ||
| forceNew: false, | ||
| autoConnect: true | ||
| }; |
There was a problem hiding this comment.
Are these the default values if left blank? If so let's not pass anything.
There was a problem hiding this comment.
I am not sure if I completely understand. The defaultOptions set certain values for some of the variables.
Where some other props for the Socket, are not set by default.
What part do you want to remove?
There was a problem hiding this comment.
the path forceNew and autoConnect already default to these values when nothing is passed to io() so no need to explicitly set them
| const socket = React.useMemo(() => io( | ||
| url || undefined, {auth: {pathname: window.location.pathname}} | ||
| ), [url]) | ||
| const { url, options = {}, setProps, eventNames } = props; |
There was a problem hiding this comment.
I'm not familiar with this syntax, will this make it an empty object only if it is null?
There was a problem hiding this comment.
It will destructure the options object. The default value is {}. If we have options {'timeout': 300} then we get that. If options: null, we would get null.
| } | ||
|
|
||
| DashSocketIO.defaultProps = {}; |
There was a problem hiding this comment.
Not really. I defined default values with
const defaultOptions: SocketOptions = { path: '/socket.io', forceNew: false, autoConnect: true };
That's why I thought it might be not needed.
There was a problem hiding this comment.
I think Dash might be expecting this to exist, at least it was part of the template, I suggest rolling this back.
| query?: Record<string, any>; | ||
| /** Authentication data */ | ||
| auth?: Record<string, any>; | ||
| [key: string]: any; |
There was a problem hiding this comment.
Let's not put this one for now.
There was a problem hiding this comment.
No just the [key: string]: any part
|
Sorry took me a while to get back to this, I answered your questions :) |
Adding path argument to dash_socketio.
Enables to customise the path and route it to different server if needed.