Skip to content

add wasmJs target #3

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 3 commits into from
Feb 7, 2025
Merged

add wasmJs target #3

merged 3 commits into from
Feb 7, 2025

Conversation

GGsrvg
Copy link
Contributor

@GGsrvg GGsrvg commented Jan 30, 2025

I added support wasm js target

@GGsrvg
Copy link
Contributor Author

GGsrvg commented Jan 31, 2025

@Piasy hi
Can u review?

@Piasy
Copy link
Member

Piasy commented Feb 5, 2025

Hi @GGsrvg , thank you for your contribution.

Actually I'm very curious how would you implement the actual log logic in wasm? I did some research earlier, but I didn't find any good ways to implement similar things as tencent's mars xlog, so I choose simple console log for js platform.

@GGsrvg
Copy link
Contributor Author

GGsrvg commented Feb 5, 2025

@Piasy
This solution uses the implementation from the common module.

  private var impl: LoggingImpl = object : LoggingImpl {
    override fun debug(tag: String, content: String) {
      println("${Clock.System.now().toEpochMilliseconds()} D $tag $content")
    }

    override fun info(tag: String, content: String) {
      println("${Clock.System.now().toEpochMilliseconds()} I $tag $content")
    }

    override fun error(tag: String, content: String) {
      println("${Clock.System.now().toEpochMilliseconds()} E $tag $content")
    }
  }

@Piasy
Copy link
Member

Piasy commented Feb 5, 2025

I mean in kmp-xlog/src/wasmJsMain/kotlin/com/piasy/kmp/xlog/Logging.kt, // using a private implementation
How would you implement it?

Any way, if you don't plan to open source your implementation, this PR shouldn't be merged, and you can create and use your own fork.

@GGsrvg
Copy link
Contributor Author

GGsrvg commented Feb 5, 2025

@Piasy I don't have my own implementation, this file is only needed to say that the implementation from the common module (which you write object : LoggingImpl) is used.

@GGsrvg
Copy link
Contributor Author

GGsrvg commented Feb 5, 2025

I wrote "private implementation" because in kmp-xlog/src/commonMain/kotlin/com/piasy/kmp/xlog/Logging.kt line 15 used object : LoggingImpl

@@ -79,109 +85,111 @@ kotlin {
}
}

mingwX64 {
Copy link
Member

Choose a reason for hiding this comment

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

Could you please revert changes below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't remove mingw, I just wrapped it in a Windows check
if (OperatingSystem.current().isWindows)

Copy link
Member

@Piasy Piasy Feb 7, 2025

Choose a reason for hiding this comment

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

You don't need to add such check, if you open the project or build the project on Linux/macOS, windows target will be disabled automatically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm... last time the check on windows allowed to success build the project
check is a removed

/**
* Created by GGsrvg{github.com/GGsrvg} on 2025/12/30.
*/
// using a private implementation
Copy link
Member

Choose a reason for hiding this comment

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

This file isn't necessary, please delete it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ready

@Piasy Piasy merged commit 6538e5e into HackWebRTC:main Feb 7, 2025
6 checks passed
@Piasy
Copy link
Member

Piasy commented Feb 8, 2025

v1.3.1 is released.

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