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

Package command - add support for multi modules #816

Merged
merged 2 commits into from
Mar 30, 2022

Conversation

lwronski
Copy link
Contributor

No description provided.

@lwronski lwronski marked this pull request as ready for review March 29, 2022 10:24
Copy link
Contributor

@alexarchambault alexarchambault left a comment

Choose a reason for hiding this comment

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

Apart from the minor comment, LGTM!

Copy link
Member

@romanowski romanowski left a comment

Choose a reason for hiding this comment

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

Shouldn't we update js documentation?

@armanbilge
Copy link
Contributor

This feature is already mentioned in the documentation, it just wasn't working :)

https://scala-cli.virtuslab.org/docs/reference/cli-options/#--js-module-split-style

@lwronski lwronski disabled auto-merge March 29, 2022 13:11
@lwronski
Copy link
Contributor Author

Shouldn't we update js documentation?

I think, that I confusingly named PR, it is more bug fixes. Because if someone run package --js command with the following scala file:

  @JSExportTopLevel(name = "onRequest", moduleID = "request_type")
  def func(...)

  @JSExportTopLevel(name = "onRequest", moduleID = "request_headers")
  def func(...)

he gets an error that main class is missing, when run generated js file. This PR fixed it.

@romanowski
Copy link
Member

That is valid, however current Scala JS guide does not have a section about publishing so I think we should describe the publishing for JS. We have a small section at publish command but I think it only covers a basic scenario.

So I am merging this PR for now, @lwronski could you review the documentation for packaging in JS and create a follow up PR if needed?

@romanowski romanowski merged commit dcbc573 into VirtusLab:main Mar 30, 2022
@armanbilge
Copy link
Contributor

Note that publishing (i.e., uploading SJSIR files to Maven) is different from packaging (linking SJSIR files into JS files).

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.

Scala.js - package command - support for multi modules
4 participants