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.

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] Capture all JS dependencies in ES module imports #193

Closed
wants to merge 6 commits into from

Conversation

JanCVanB
Copy link
Collaborator

@JanCVanB JanCVanB commented Jun 2, 2022

TODO:

  • Manually fix remaining lint errors
  • Wait for Clean up various code smells #189 to merge (currently has errors that prevent full testing of this branch)
  • Fully test this branch with my local Xpra server

@JanCVanB JanCVanB force-pushed the imports branch 4 times, most recently from 0b1d392 to 6758a97 Compare June 4, 2022 08:02
@totaam
Copy link
Collaborator

totaam commented Jun 5, 2022

The libraries should be excluded from the split, ie: mitm.html

@JanCVanB
Copy link
Collaborator Author

JanCVanB commented Jun 5, 2022

The libraries should be excluded from the split, ie: mitm.html

Oh, could you please explain what mitm does? And what "libraries" means in this context?

@totaam
Copy link
Collaborator

totaam commented Jun 6, 2022

Oh, could you please explain what mitm does?

I don't really understand it well, it is part of StreamSaver.js - added in ce0bc83
It goes with sw.js and it should be possible to move them to /js/lib somehow but managing relative paths quickly becomes tricky so I didn't attempt it.
Many will deploy xpra to relative paths and we have to make sure things still work there.

And what "libraries" means in this context?

Anything that is going to be updated independently of xpra.

@JanCVanB
Copy link
Collaborator Author

JanCVanB commented Jun 6, 2022

@totaam Ah, now I understand mitm, thanks. I'll revert its modifications here, so that it remains maintainable.

The libraries should be excluded from the split

Do you mean that this PR shouldn't touch html5/lib/js/ files? If so, then this PR is impossible, since those files currently "export" their variables/functions to the global window context - they either need light (maintainable) modification to become compatible with ESM import syntax or they need to be replaced entirely by npm dependencies.

@totaam
Copy link
Collaborator

totaam commented Jun 6, 2022

they either need light (maintainable) modification to become compatible with ESM import syntax or they need to be replaced entirely by npm dependencies

I would prefer the former.
Can we keep most of the formatting from their upstream source?
(to make it easier to diff with upstream)

@JanCVanB
Copy link
Collaborator Author

JanCVanB commented May 8, 2023

Closing for staleness

@JanCVanB JanCVanB closed this May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants