ClassDef checker error after linking when only using the class data of a non-native JS class #4850
Comments
|
Hum, interesting. FWIW, the bug probably existed before 1.13.1, but did not do ClassDef checks after the optimizer before. You should be able to work around the issue by disabling IR checks ( |
|
Alternative workaround: add a scala.scalajs.js.constructorOf[sttp.tapir.serverless.aws.lambda.js.AwsJsRequestContext]somewhere in your code (not in dead code), such as in your |
|
Thanks for the quick reply! From a quick grep for It seems that the IR-checks are enabled for I'll test out the workaround and report back as well. |
|
I can confirm two things already:
I didn't manage to find how I enable |
|
Ah yes, it's true by default in fullLink. I had forgotten about that. The correct setting is: .settings(Compile / fullLinkJS / scalaJSLinkerConfig ~= { _.withCheckIR(false) } |
|
Thanks, then I can confirm that disabling the IR check that way also works around the issue! Just to have an idea of how long a workaround would be in our codebase until a fix is released in scala-js: Will this bug be reason for a quick release of 1.13.2 since it's (technically) a regression, or will it simply be left as-is since it might not affect many projects and has an easy workaround? Again, thanks for the quick replies! :) |
|
It's unlikely that we'll make a quick release for this, unless a number of other people experience the same issue. We normally stick to our 2-month schedule. |
|
Alright, thanks for letting me know, and for the quick replies again! |
…their data. This underlying issue was already fixed in ae17b93.
…their data. The underlying issue was already fixed in ae17b93.
…their data. The underlying issue was already fixed in ae17b93.
…d JS types. When a JS type is not instantiated, it loses its constructor, which makes it invalid from the ClassDef checker's perspective. We now turn such JS types into `AbstractJSType`s, which do not need any constructor. For native JS types, this also means that they should get rid of their `jsNativeLoadSpec`, since abstract JS types must not have one.
…d JS types. When a JS type is not instantiated, it loses its constructor, which makes it invalid from the ClassDef checker's perspective. We now turn such JS types into `NativeJSClass`es that do not have a native load spec.
sjrd
changed the title
There were several interleaved issues with JS classes (native or not, module or not) that survived the base linker only for their class data: - Non-native classes lose their `jsConstructorDef` (which is what caused the symptoms in the issue). - Non-native non-module classes generate a `Class.isInstance` test that tries to access a never-declared `$a_C()` accessor. - Module classes lose their module status, triggering the wrong conditions for the `Class.isInstance` tests later on. Some related issues about module accessors were previously handled by patching `ClassKind`s. For module classes whose module accessor was not used, we patched the kind to a corresponding kind without module accessor. This was problematic as well: it caused the third issue above, as well as making the kind unstable over incremental runs (although we tend to assume that the kind is covered by the class version). We solve all these issues at once, by taking a much more principled approach: - We never change the kind of classes. - After base linking, we tolerate `ClassDef`s without constructors even if they are module classes and/or non-native JS classes. - In the emitter, we adjust the generation of module accessors and `Class.isInstance` tests to whether the class effectively has instances.
There were several interleaved issues with JS classes (native or not, module or not) that survived the base linker only for their class data: - Non-native classes lose their `jsConstructorDef` (which is what caused the symptoms in the issue). - Non-native non-module classes generate a `Class.isInstance` test that tries to access a never-declared `$a_C()` accessor. - Module classes lose their module status, triggering the wrong conditions for the `Class.isInstance` tests later on. Some related issues about module accessors were previously handled by patching `ClassKind`s. For module classes whose module accessor was not used, we patched the kind to a corresponding kind without module accessor. This was problematic as well: it caused the third issue above, as well as making the kind unstable over incremental runs (although we tend to assume that the kind is covered by the class version). We solve all these issues at once, by taking a much more principled approach: - We never change the kind of classes. - After base linking, we tolerate `ClassDef`s without constructors even if they are module classes and/or non-native JS classes. - In the emitter, we adjust the generation of module accessors and `Class.isInstance` tests to whether the class effectively has instances.
…lass-data-2 Fix #4850: Handle JS classes that survive only for their data.
In a project (non-public :( ) that uses tapir, compilation with 1.13.0 worked fine, while the upgrade to 1.13.1 throws the following error:
The offending line seems to be this one:
https://github.com/softwaremill/tapir/blob/master/serverless/aws/lambda/src/main/scalajs/sttp/tapir/serverless/aws/lambda/js/AwsJsRequest.scala#L29
No idea how to minimize this, but since it's a public library, it should be possible to reproduce. This bug occurs when compiling tapir 1.2.12, which is also the most recent release, released last week (at the time of writing).
Please let me know if/what further information can help, or to test a possible fix! Really looking forward to those incremental-comp performance upgrades in 1.13.1 :) Thanks!
The text was updated successfully, but these errors were encountered: