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 js exports generation #844

Merged
merged 5 commits into from
Dec 20, 2023
Merged

Add js exports generation #844

merged 5 commits into from
Dec 20, 2023

Conversation

eymar
Copy link
Member

@eymar eymar commented Dec 12, 2023

This contribution was made by @ilgonmic ! Thank you Ilya!

Description:
To ensure that setup.mjs file contains correct exports, we better generate it than maintain manually.
It's done by a compiler plugin, which looks at WasmImport annotations and takes the name value from it.

P.S. our build.gradle.kts is quite big already (~1.8k lines). I tried to move new part to buildSrc, but I didn't manage to make work. I'll try to think about it separately.

UPD: a separate PR to split the build.gradle.kts into smaller parts - #845

@eymar eymar changed the title Add js imports generation Add js exports generation Dec 12, 2023
@eymar eymar requested review from igordmn and Schahen December 12, 2023 17:14
outputs.file(skikoTestMjs)
}

listOf(main, test).forEach {
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 this piece at all?

Copy link
Member Author

Choose a reason for hiding this comment

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

I rebased on master, therefore this comment doesn't link anymore to the code.
This part is indeed needed:
KotlinCompilerPluginSupportPlugin has a method fun getPluginArtifact() which we override. We return SubpluginArtifact(SkikoArtifacts.groupId, IMPORT_GENERATOR) with null version. Therefore by default it tries to get the same version as koltin version.

Since our compiler plugin is not published, we need to substitute this implicit dependency.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also I moved it to buildSrc

}

abstract class AbstractImportGeneratorCompilerPluginSupportPlugin(
Copy link
Collaborator

Choose a reason for hiding this comment

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

it seems it might sense to keep this in buildSrc

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved to buildSrc.

@eymar eymar force-pushed the ok/js_exports_generation branch from 78fba00 to 23b3541 Compare December 19, 2023 11:59
@eymar eymar merged commit dcc7cfe into master Dec 20, 2023
5 checks passed
@eymar eymar deleted the ok/js_exports_generation branch December 20, 2023 16:31
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.

3 participants