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

Add Kotlin asset support #2210

Merged
merged 4 commits into from Dec 18, 2018
Merged

Conversation

Zoweb
Copy link
Contributor

@Zoweb Zoweb commented Oct 28, 2018

↪️ Pull Request

This should close #2177 by adding support for Kotlin. Unfortunately, the only possible way to implement Kotlin support was to use the compiler, which can only generate files, so a temporary directory is created and deleted with those files, if there is nothing else in it.

Running Kotlin functions from other languages (e.g. importing a Kotlin file in a JavaScript file, then running a function defined in it) doesn't work as intended - the Kotlin compiler adds some extra text to the function names, which has to be added in the other language as well.

To get Kotlin code to run automatically, instead of just putting the code directly in the file, create a fun main(args: Array<String>). This function will be called with an empty list of arguments.

Note that the Kotlin standard library is included in the result, and is huge. Building in production mode minifies the source but it is still over 600KB with the test described below. Implementing treeshaking may help, if this is not already done.

🚨 Test instructions

Create an index.html file and a test.kt file. Add a <script> tag with the src pointing to the kt file.

Inside the Kotlin file, add the following code:

fun main(args: Array<String>) {
    for (i in 0 until 100) {
        if (i % 20 == 0) print("FooBar")
        else if (i % 5 == 0) print("Bar")
        else if (i % 4 == 0) print("Foo")
        else print(".")

        println(" [$i]")
    }
}

This should print 100 lines, where every 4th is "Bingle [(line num)]", 5th is "Foo [(line num)]", 20th is "FooBar [(line num)]" and every other line is ". [(line num)]"

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

@Zoweb Zoweb mentioned this pull request Oct 28, 2018
@Zoweb
Copy link
Contributor Author

Zoweb commented Oct 30, 2018

Hmm, it looks like elm failed but just on Windows/Node 6

// remove extension and path
let slashyIndex = this.id.lastIndexOf('/'); // linux/mac
if (slashyIndex === -1) slashyIndex = this.id.lastIndexOf('\\'); // windows
const fileName = this.id.substring(slashyIndex, this.id.lastIndexOf('.'));
Copy link
Member

Choose a reason for hiding this comment

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

Can you use the path library for this? It will take care of the normalization cross platform for you.

const fileName = this.id.substring(slashyIndex, this.id.lastIndexOf('.'));

await kotlinCompiler.compile({
output: 'build/kt.temp/' + fileName + '.js',
Copy link
Member

Choose a reason for hiding this comment

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

You should be explicit about exactly where this is going to go. Right now it will go into the current working directory. Instead, it would be better to go into a temporary directory.

@devongovett
Copy link
Member

This is awesome! It would be great if you could add some tests.

@devongovett devongovett merged commit 29dca2f into parcel-bundler:master Dec 18, 2018
@Zoweb
Copy link
Contributor Author

Zoweb commented Dec 18, 2018

Oh thanks for doing those changes! I only saw this yesterday, so hadn't had time to do it yet.

@ghost ghost mentioned this pull request Dec 21, 2018
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.

Kotlin support
2 participants