Feature/source map support #1710
Conversation
linusha
force-pushed
the
feature/source-map-support
branch
from
2c0c30a to
df6e577
Compare
merryman
force-pushed
the
feature/source-map-support
branch
from
df6e577 to
a0c0f2a
Compare
There was a problem hiding this comment.
Alright, I took the liberty of removing some (very few) unused lines of code.
I also raised some points that I found to be concerning below.
I checked the tests, one fails:
Additionally, I developed a bit inside of lively and everything worked. I also checked building a project (the website), which worked. However, one thing on the website is off (bottom most picture):
If you are sure this is unrelated to these changes, that's also fine with me. We should however note that somewhere and fix it.
Pending the issues raised below and the failing test, this seems good to me, @merryman. Any other pointers as to where I should apply some pressure to check whether this is good to go?
| "rollup": "4.27.3", | ||
| "@rollup/rollup-linux-x64-gnu": "4.27.3", | ||
| "@rollup/rollup-darwin-arm64": "4.27.3", |
There was a problem hiding this comment.
Do we risk problems here on windows machines?
There was a problem hiding this comment.
We may better test that manually on our trusty windows setup, but doesn't windows use this obscure linux subsystem which should be fine with linux-x64?
There was a problem hiding this comment.
That might be how it works, I don't remember anymore from the top of my head honestly, what happens there under the hood. I'll note that down and test it eventually though!
This issue appears on |
merryman
force-pushed
the
feature/source-map-support
branch
from
15eeb91 to
0ed2d5f
Compare
Verifying the debugging with source-maps works OK, would be a good test ^^. |
Merge the var and ref replacement and the import export traversal into single passes.
The key is here to make the node functions parametrized, so that depending on the underlying AST system different nodes can be created by the same transform.
Was needed due to to the changes in the class transform that preserves the class name as a binding during class initialization.
Since babel.js has a bunch of circular imports it has weird importing specific code that checks if modules have been already initialized. We introduce a method called bulletProofNamespace() which helps us to adjust these error checking codes such that they work in split bundles.
The transpiler initialization needed to be adjusted.
merryman
force-pushed
the
feature/source-map-support
branch
from
0ed2d5f to
4a40a38
Compare
linusha
force-pushed
the
feature/source-map-support
branch
from
4a40a38 to
63f0d90
Compare
linusha
left a comment
linusha
left a comment
There was a problem hiding this comment.
Verifying the debugging with source-maps works OK, would be a good test ^^.
I did that! And it works :)
I will keep testing this on a windows machine on my list and hopefully do that next week. Regardless, my proposal would be to continue merging this for now, and afterwards also merging #1711.
Can be merged once #1704 is merged.