Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

if it possible to extract data provider out of component? #32

Closed
timzaak opened this issue May 28, 2024 · 13 comments · Fixed by #72
Closed

if it possible to extract data provider out of component? #32

timzaak opened this issue May 28, 2024 · 13 comments · Fixed by #72
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed
Milestone

Comments

@timzaak
Copy link

timzaak commented May 28, 2024

I use MQTT to get log line by line, it's not websocket nor http.

it would be nice if the api look like:

interface LazyLogController {
  newLine(lines:[]string) => void
  end() => void
  start() => void
  error() => void
}

const ref = userRef<LazyLogController>()

useEffect(() => {
   ref.current?.start()
   ref.current?.newLines(['test1','test2'])
   // ...
}, [])

<LazyLog ref={ref} follow>
@melloware
Copy link
Owner

PR's are welcome @timzaak. I forked this library because it had gone dead and had a bunch of bugs.

@melloware melloware added enhancement New feature or request help wanted Extra attention is needed labels May 28, 2024
@melloware
Copy link
Owner

It looks like MQTT supports websocket?

https://www.hivemq.com/blog/mqtt-essentials-special-mqtt-over-websockets/

@timzaak
Copy link
Author

timzaak commented May 31, 2024

Yes, but mqtt.js API has different API。

@melloware
Copy link
Owner

@timzaak are you interested in creating a PR for this feature?

@timzaak
Copy link
Author

timzaak commented Jun 7, 2024

@timzaak are you interested in creating a PR for this feature?

ihave tried,but this would break api compatible(move state manager outside of component,user solve fetch data themselves,provide method to render lines)

@melloware
Copy link
Owner

@timzaak see the PR what about using EventSource instead of WebSockets?

@timzaak timzaak closed this as completed Nov 1, 2024
@JokerQyou
Copy link
Contributor

JokerQyou commented Dec 5, 2024

I need this too. Currently I maintain an EventSource built by third-party library (because the standard EventSource does not support custom Authorization header), so have to use the text prop to pass log content to <LazyLog />. But this affects performance:

  • Log data is maintained twice: I have lines stored in state, and I joins them into a string passed to text prop, and <LazyLog /> parses them again and stored a second copy of these identical lines. So memory usage doubles.
  • When new log lines get appended, <LazyLog /> is re-rendered, the UI would flash, because all log lines are processed again from the beginning, and follow handling has to start over.

Ideally there would be a pattern like this:

  • My component holds a ref to the <LazyLog /> child: const logViewer = useRef<LazyLog>();
  • Each time a new log line is received, I just call logViewer.current?.appendLine(line). My component does not need to store this new line, and <LazyLog /> only needs to render the new line.

@melloware
Copy link
Owner

@JokerQyou you can't use the new event source code? #51 ?

@JokerQyou
Copy link
Contributor

No. EventSource in browser does not support customizing headers, that's a known limitation with both the current standard and implementations, see whatwg/html#2177. Some libraries like SSE.js and fetch-event-source use XHR or fetch with keep-alive connections to simulate the native EventSource, but I don't think react-logviewer would support them.

@melloware
Copy link
Owner

Understood. Well a PR is welcome if you can get it working but extracting some of this stuff out is not easy when I looked. I think even adding an logViewer.current?.appendLine(line) could be doable but will still trigger a re-render i think.

@JokerQyou
Copy link
Contributor

JokerQyou commented Dec 5, 2024

I worked around by hacking with logViewer.current?.setState(...) since it's a class-based component 😂 . Should it be a function component it would not be possible. I'll look into adding appendLine support tomorrow. Hopefully it can be achieved.

@melloware
Copy link
Owner

That would be awesome! Looking forward to it. Yeah this original component was written back in 2018 before Function Compoents and so I have tried to touch as little as possible when taking it over after the original project was abandoned!

Thanks in advance for the PR!

@JokerQyou
Copy link
Contributor

See #72 .

@melloware melloware reopened this Dec 6, 2024
@melloware melloware added this to the 6.0.4 milestone Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants