Skip to content

Scala.js: Implement JS exports. #10164

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 2 commits into from
Nov 12, 2020
Merged

Conversation

sjrd
Copy link
Member

@sjrd sjrd commented Nov 3, 2020

Compared to the implementation in Scala 2, there is one significant difference. In Scala 2, top-level exports and static exports info are computed by PrepJSInterop and stored in an out-of-band mutable Map until the back-end. In this implementation, PrepJSInterop only performs checks for compile error messages, and the required data are recomputed by the back-end from the annotations.

Since dotc does not (currently?) constant-fold expressions within annotations, PrepJSInterop specifically constant-folds the arguments of @JSExportTopLevel and @JSExportStatic, so that the literal values can be recovered by the back-end.

Copy link
Member

@dottybot dottybot left a comment

Choose a reason for hiding this comment

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

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Commit Messages

We want to keep history, but for that to actually be useful we have
some rules on how to format our commit messages (relevant xkcd).

Please stick to these guidelines for commit messages:

  1. Separate subject from body with a blank line
  2. When fixing an issue, start your commit message with Fix #<ISSUE-NBR>:
  3. Limit the subject line to 72 characters
  4. Capitalize the subject line
  5. Do not end the subject line with a period
  6. Use the imperative mood in the subject line ("Add" instead of "Added")
  7. Wrap the body at 80 characters
  8. Use the body to explain what and why vs. how

adapted from https://chris.beams.io/posts/git-commit

Have an awesome day! ☀️

@sjrd
Copy link
Member Author

sjrd commented Nov 4, 2020

I added a commit that adds neg tests and makes them pass. It should be squashed eventually, so if you haven't started reviewing, it's better to review the PR diff than the commits separately.

@sjrd sjrd force-pushed the sjs-js-exports branch 2 times, most recently from 02c8e4a to 48fdd56 Compare November 5, 2020 17:16
sjrd added 2 commits November 11, 2020 16:30
Compared to the implementation in Scala 2, there is one significant
difference. In Scala 2, top-level exports and static exports info
are computed by `PrepJSInterop` and stored in an out-of-band
mutable Map until the back-end. In this implementation,
`PrepJSInterop` only performs checks for compile error messages,
and the required data are recomputed by the back-end from the
annotations.

Since dotc does not (currently?) constant-fold expressions within
annotations, `PrepJSInterop` specifically constant-folds the
arguments of `@JSExportTopLevel` and `@JSExportStatic`, so that the
literal values can be recovered by the back-end.
@sjrd
Copy link
Member Author

sjrd commented Nov 11, 2020

Rebased, squashed, and added prepjsinterop in the phases to Ycheck, now that #10116 is fixed.

} else /*if (jsInterop.topLevelExportsOf(sym).nonEmpty) {
val f = js.SelectStatic(encodeClassName(sym.owner),
encodeFieldSym(sym))(jstpe.AnyType)
} else if (sym.hasAnnotation(jsdefn.JSExportTopLevelAnnot)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you recall why we didn't simply do this in Scala 2?

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 don't know. It's probably historical, following a long chain of changes. @JSExportTopLevel inherited from @JSExport on classes and objects, in which the name could be implicit; which might explain it. git blame suggests scala-js/scala-js@273e9b8 as the moment at which we introduced those out-of-the-band mutable maps, but at the time @JSExportTopLevel did not exist yet.

We could try to get rid of it in Scala 2.

@sjrd sjrd merged commit 49e853c into scala:master Nov 12, 2020
@sjrd sjrd deleted the sjs-js-exports branch November 12, 2020 08:50
@Kordyjan Kordyjan added this to the 3.0.0 milestone Aug 2, 2023
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.

6 participants