Skip to content

fix: package compatibility with web build #138

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

mohitpubnub
Copy link
Contributor

@mohitpubnub mohitpubnub commented Aug 13, 2025

fix: compatibility issue with web build

Fixes package compatibility issues with web builds.

refactor: allow custom logger based on ILogger.

Allow global custom logger to be ILogger based.

refactor: upgrade dependencies.

upgrade outdated dependencies.

Copy link
Contributor

@parfeon parfeon left a comment

Choose a reason for hiding this comment

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

Mostly looks good!
I have question how optional boolean will look like if not defined? false by default?

'membersMetadata': membersMetadata,
};
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we have @override here?

Copy link
Contributor Author

@mohitpubnub mohitpubnub Aug 13, 2025

Choose a reason for hiding this comment

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

Since toJson() is not universal and it’s a per-class convention it's not an overridden method.

Also, boolean without assigned in all parameters will be printed null,
and while preparing a url, if those values optional then they are omitted.

@mohitpubnub mohitpubnub self-assigned this Aug 13, 2025
Mohit Tejani added 2 commits August 13, 2025 21:28
keeping default logger enabled when custom logger provided.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants