Skip to content

react-native: replace AlternatingFileWriter with WritableStream and ChunkifierSink for breadcrumbs #315

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

Merged
merged 11 commits into from
Nov 18, 2024

Conversation

perf2711
Copy link
Contributor

This PR replaces AlternatingFileWriter with WritableStream and ChunkifierSink based on chunkifier from Node.
As React Native does not have a WritableStream support by default, a ponyfill is used.

@perf2711 perf2711 added bug Something isn't working enhancement New feature or request labels Nov 12, 2024
@perf2711 perf2711 self-assigned this Nov 12, 2024
@perf2711 perf2711 force-pushed the bugfix/BT-5147-react-native-breadcrumbs branch from 545f249 to 1ad7f87 Compare November 12, 2024 11:52
public async write(data: W): Promise<void> {
// If data is empty from the start, forward the write directly to current stream
if (this.isEmpty(data)) {
return await this.ensureStreamContext().streamWriter.write(data);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to run a write operation at all? Based on the stream writer implementation, we don't need to?

Copy link
Contributor Author

@perf2711 perf2711 Nov 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just that any write to this sink should end up in calling write on the underlying stream.

@perf2711 perf2711 force-pushed the bugfix/BT-5147-react-native-breadcrumbs branch from 0872be8 to 414e2a9 Compare November 15, 2024 11:13
@perf2711 perf2711 merged commit 8ab904a into main Nov 18, 2024
5 checks passed
@perf2711 perf2711 deleted the bugfix/BT-5147-react-native-breadcrumbs branch November 18, 2024 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants