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

Request for multiplatform #3

Open
NikkyAI opened this issue Jan 12, 2020 · 12 comments
Open

Request for multiplatform #3

NikkyAI opened this issue Jan 12, 2020 · 12 comments

Comments

@NikkyAI
Copy link
Contributor

NikkyAI commented Jan 12, 2020

Is it possible to package this for multiplatform?

for my own project i just copied your code into my commonMain

i use this with kotlin/js and react, but having it in common code leaves the option open to render stuff serverside too

seems like Chat.isUpperCase() does not exist on js so i solved it expect/actual, iirc this is the only problem i ran into

//common
expect val Char.isUpperCase: Boolean

// js
actual val Char.isUpperCase: Boolean
    get() = (this == this.toUpperCase() && this != this.toLowerCase())

// jvm
actual val Char.isUpperCase: Boolean
    get() = isUpperCase()
@nwillc
Copy link
Owner

nwillc commented Jan 12, 2020 via email

@nwillc
Copy link
Owner

nwillc commented Jan 13, 2020

I assume the logging support will need fixing too.

@NikkyAI
Copy link
Contributor Author

NikkyAI commented Jan 13, 2020

@nwillc
Copy link
Owner

nwillc commented Jan 14, 2020 via email

@nwillc
Copy link
Owner

nwillc commented Jan 30, 2020 via email

@NikkyAI
Copy link
Contributor Author

NikkyAI commented Jan 31, 2020

i assume with js specific code you mean the logging?

it looks like it will work
but i think it will not show the original file location / line numbers when using the browser dev tools
for that iirc the logger calls need to all be inline or so, never looked into it in detail
korlibs/klogger works for me, but it has no decent formatting

also 1.3.70-eap had some mindblowing improvements to the js side of things
although it is fine for this, since you are depending on barely anything

through experimentation i found out that this or something similar is required in the buildscript

js() {
        useCommonJs()
        browser {
            runTask {
                sourceMaps = true
            }
            webpackTask {
                sourceMaps = true
            }
        }
        compilations.all {
            kotlinOptions {
                sourceMap = true
                metaInfo = true
                main = "call"
            }
        }
        mavenPublication { // Setup the publication for the target
//            artifactId = "ksvg-js"
            // Add a docs JAR artifact (it should be a custom task):
//            artifact(javadocJar)
        }
    }

without it the project using it will complain that ksvg is not configured for js
this error only nees to pop up when it is included as subproject, but after fixing it using it as a library from mavenLocal seems to work too

@nwillc
Copy link
Owner

nwillc commented Feb 1, 2020

I've accepted your PR. It's not the logging I'm worried about. I don't have a JS test harness to try things with and am struggling getting JS unit testing going.

@NikkyAI
Copy link
Contributor Author

NikkyAI commented Feb 1, 2020

i can look into that

also have similar problems of not knowing how to run tests in the browser

@NikkyAI
Copy link
Contributor Author

NikkyAI commented Feb 1, 2020

seems like this is breaking things.. https://stackoverflow.com/questions/51475050/spaces-in-test-method-names-when-targeting-multiplatform

TL;DR: cannot have spaces in testnames in commonTest because it cannot compile to js

@NikkyAI
Copy link
Contributor Author

NikkyAI commented Feb 1, 2020

after fixing all the testnames with @JsName

these errors are left over

here jvm and js seem to sort x1, x2, y1, y2 differently

"<svg>\n<line y1=\"$y1Value\" x1=\"$x1Value\" y2=\"$y2Value\" x2=\"$x2Value\"/>\n</svg>\n"

IllegalArgumentException: Value (10) is not a valid length or percentage

IllegalArgumentException: Value (M 10,30 A 20,20 0,0,1 50,30 Z) is not a valid path

in addition to that there seem to be a handful failures that look like similar regex or sorting failures
https://github.com/nwillc/ksvg/blob/c511c26534111ea9882eefea4256194ef12b7f0e/src/commonTest/kotlin/com/github/nwillc/ksvg/attributes/AttributeTypeTest.kt

for the last few errors it seems like the regex might be acting up, i will open a PR again for the current progress

This was referenced Feb 1, 2020
@nwillc
Copy link
Owner

nwillc commented Feb 8, 2020

The issues in the tests I believe are resolved. The sort order is now enforced and the other bits and pieces seem resolved. The JVM build artifacts now work with existing clients so I've merged the branch to master. I think the javascript is working but I still don't understand the packaging and how it would be used in an example. More to do there.

@harryjph
Copy link
Contributor

Please see #10 for instructions on how to use this in a multiplatform project

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

No branches or pull requests

3 participants