Use opaque types for Scala 3 #592
Conversation
| object IDBTransactionDurability { | ||
| @inline def default: IDBTransactionDurability = "default" | ||
| @inline def strict: IDBTransactionDurability = "strict" | ||
| @inline def relaxed: IDBTransactionDurability = "relaxed" |
There was a problem hiding this comment.
Is there any reason for some of the constants to be @inline defs instead of vals? It seems that it would be nicer if all of them were vals, doesn't it?
There was a problem hiding this comment.
Agreed, I'll take a pass through and clean these up.
There was a problem hiding this comment.
Ah that's me, or if it wasn't me, that's something I do. My reasoning is that it reduces unnecessary bytecode. If they're vals they're always present in the output JS (so long as the object is instantiated), where as with defs unused fields can be omitted.
There was a problem hiding this comment.
Ah, that's a good point. I think the advantage of vals is that they are stable and can be used for matching, but not sure how much use that is here. Happy to go either way.
There was a problem hiding this comment.
Is there any way these get inlined e.g. if they are final val?
There was a problem hiding this comment.
Btw: just noting that if a future release of Scala.js gets some kind of purity checking it will be able to remove unused vals from the output too. It wouldn't even need to be complex to detect cases like this. @sjrd: I'm might just ping to put it back on your radar :)
There was a problem hiding this comment.
If you used inline val you would also end up with no field in the bytecode.
There was a problem hiding this comment.
Good to know, can we still make this change binary-compatibly?
There was a problem hiding this comment.
I don't know well enough how binary-compatibility works in the Scala.js world so I can't say :).
There was a problem hiding this comment.
I don't know well enough the behavior of inline val in terms of compilation scheme so I can't say :).
We'll have to try.
Closes #587. Note: basing against #588 for now.