-
Notifications
You must be signed in to change notification settings - Fork 0
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
[WIP] - Update 'from' imports #10
base: master
Are you sure you want to change the base?
Conversation
Destructuring might not be the worst option. You would want the transform to look more like: import device from 'device';
const { hideAddressBar: hab } = device; I think destructuring is fine for the automatic mods. It doesn't look great, but we will end up doing 90% by hand if we don't start accepting some of these edge cases. Depending on how many of these cases there are, it might make the most sense to add another mod for restructuring exports / imports. We could then run that on a case by case basis. I am going to get @fairfieldt input on this as well. |
We want to convert all of the timestep code to the idiomatic style: This seems pretty manageable as a manual process if there's no way to do it automatically. For the script to convert a game's code, the restructuring looks fine to me |
It sounds like we want to add an environment variable |
@yofreke that sounds like a nice to have, v2 of these mods I'm thinking. Whaddayathink? |
@@ -0,0 +1,3 @@ | |||
from device import hideAddressBar as hab; | |||
|
|||
hideAddressBar(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would this one throw an error?
Adding that as a v2 sounds fine. First deliverable should be the strict variant (for use with library code). Just keep this in mind when writing things, as to not make it too complicated to add later. |
Because we want to bundle all exported variables in the default export we need to consider the same for imports.
Right now, this bit of jsio syntax:
compiles to this:
which quite obviously won't work because hab is bound to the default export.
There's no compatible transform for this really, so instead we need to import the module and then destructure the
from
:not super pretty but the only way I can see rn. cc @yofreke