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.

Sign up for GitHub

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

Jump to bottom

Fix #19528: Actually remove Dynamic from interfaces of native JS classes. #19536

Merged
merged 1 commit into from
Jan 30, 2024
Merged

Fix #19528: Actually remove Dynamic from interfaces of native JS classes. #19536

merged 1 commit into from
Jan 30, 2024

Conversation

sjrd
Copy link
Member

@sjrd sjrd commented Jan 26, 2024

One boolean value was the wrong way around for native JS classes and traits. That caused scala.Dynamic not to be removed from the super-interfaces of native JS classes at the IR level, causing the linking error.

@sjrd
Fix scala#19528: Actually remove Dynamic from interfaces of native JS…
59ebb85 
… classes.

One boolean value was the wrong way around for native JS classes
and traits. That caused `scala.Dynamic` not to be removed from the
super-interfaces of native JS classes at the IR level, causing the
linking error.
@sjrd sjrd requested a review from nicolasstucki January 26, 2024 08:31
@sjrd sjrd linked an issue Jan 26, 2024 that may be closed by this pull request
Cannot use Dynamic with native Scala.js class / trait #19528
Closed
@nicolasstucki nicolasstucki merged commit aac902e into scala:main Jan 30, 2024
19 checks passed
@nicolasstucki nicolasstucki deleted the sjs-fix-custom-dynamic-i19528 branch January 30, 2024 13:41
@Kordyjan Kordyjan added this to the 3.4.1 milestone Feb 14, 2024
@OndrejSpanel
Copy link
Member

I think this fix should be back-ported. The issue makes some Scala.js projects which worked in Scala 2 not buildable, with no good workaround.

The actual fix is change of a single boolean literal used for forJSClass parameter from false to true, therefore porting it should hopefully be trivial. The added tests are also straightforward, in a new file.

@OndrejSpanel
Copy link
Member

Actually if I knew what branch should I target, I would gladly try preparing the backport PR.

@OndrejSpanel
Copy link
Member

All other issues in Scala 3 I have reported have some workaround in my project, except this one - this issue is blocking me to move to Scala 3. It seems 3.4.1 release is just around a corner now, but that is Scala-next - going straight to it means manually disabling a bunch of new deprecation warnings.

When can Scala 3 LTS release with this fix (hopefully 3.3.4) be expected? Can I do something to expedite it?

The only info about upcoming LTS release I have found is https://users.scala-lang.org/t/scala-3-how-do-we-make-a-lambda-type-conform-to-a-subtype/9805/2:

The earliest the fix can be applied is 3.3.3 which is ~6 weeks from now.

This was Feb 16, 4 weeks ago.

I would really hate 3.3.4 to be released without this fix - and I am a bit worried about the silence on the LTS release front.

@sjrd sjrd added the backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. label Mar 19, 2024
@sjrd
Copy link
Member Author

sjrd commented Mar 19, 2024

You might help get it backported by submitting a PR with a cherry-pick to the release-3.3.4 branch.

I also added the backport:nominated label, just in case.

@OndrejSpanel OndrejSpanel mentioned this pull request Mar 19, 2024
Merged
@OndrejSpanel
Copy link
Member

OndrejSpanel commented Mar 19, 2024 •
edited
Loading

I am a bit confused about Scala 3.3 LTS backports project - as I see it, the project is currently empty. Is the project used at all? I have added this PR into a column I think it belongs to, but I was quite surprised to see I am allowed to freely move the items around or even to edit workflows or to close the project if I want to. Am I supposed to have the access rights needed for this?

@OndrejSpanel
Copy link
Member

And now there are no projects in https://github.com/orgs/scala/projects at all, open or closed. Confused even more. 😶

@OndrejSpanel
Copy link
Member

There is now https://github.com/orgs/lampepfl/projects/6 in which this PR is in the column Needs Assessment

I would say it belongs to PR Submitted and could hopefully move to Backport Done soon, but there is little I can do to make it happen - I am not able to make any changes in that project which is how it should be.

sjrd added a commit that referenced this pull request Apr 8, 2024
@sjrd
Backport fix #19536 of Scala.js Dynamic (#19979)
372f0fa 
Backport cherry-pick, as discussed in #19536
@WojciechMazur WojciechMazur mentioned this pull request Jun 28, 2024
Merged
WojciechMazur added a commit that referenced this pull request Jul 1, 2024
@WojciechMazur
Backport "Fix #19528: Actually remove Dynamic from interfaces of nati…
56cb527 
…ve JS classes." to LTS (#20866)

Backports #19536 to the LTS branch.

PR submitted by the release tooling.
[skip ci]
@WojciechMazur WojciechMazur removed the backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. label Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolasstucki nicolasstucki nicolasstucki approved these changes

Assignees

@nicolasstucki nicolasstucki

Labels
None yet
Projects
None yet
Milestone
3.4.1
Development

Successfully merging this pull request may close these issues.

Cannot use Dynamic with native Scala.js class / trait
5 participants
@sjrd @OndrejSpanel @nicolasstucki @Kordyjan @WojciechMazur