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

Would you accept a PR to drop the collections package? #234

Closed
vicb opened this issue May 31, 2024 · 7 comments
Closed

Would you accept a PR to drop the collections package? #234

vicb opened this issue May 31, 2024 · 7 comments

Comments

@vicb
Copy link
Contributor

vicb commented May 31, 2024

Integrating igc-xc-score in flyxc.app breaks the app.

The root cause is the collections package:

Design principles: extends core types (e.g extends Array.prototype with additional non-enumerable properties like .set)

I can create a PR to replace it with another package not polluting the global namespace.

Would that be ok?

Ref: vicb/flyXC#203

cc @flyingtof

@mmomtchev
Copy link
Owner

I removed the Map which was no longer needed, SortedSet is required however, if you can find another package that implements a sorted set, sure

@vicb
Copy link
Contributor Author

vicb commented Jun 3, 2024

Cool, thank you so much.

What about https://www.npmjs.com/package/@rimbu/sorted?

It seems popular enough, maintained, and clean code.
It works on the most popular runtime both server side and client side.

@laggery
Copy link

laggery commented Jun 17, 2024

Hi @vicb

Have you already started the implementation?
I have the same need and maybe I could help you with the implementation.
Best regards
Yannick

@vicb
Copy link
Contributor Author

vicb commented Jun 17, 2024

@laggery not yet.

For now we did import igc-xc-score in a web worker to isolate the side effects.

Our plan is to fork this package into https://github.com/vicb/flyxc and clean the package there (and publish to npm from there)

@laggery
Copy link

laggery commented Jun 17, 2024

@vicb Thank you very much for your quick reply.

To be able to finish my update, I will try the web worker workaround.

@vicb
Copy link
Contributor Author

vicb commented Jun 17, 2024

You can check apps/fxc-front/src/app/workers/optimizer.ts and where it is loaded in flyxc

@vicb
Copy link
Contributor Author

vicb commented Sep 27, 2024

Closing as the SortedSet does not look to pollute the global namespace.

@vicb vicb closed this as completed Sep 27, 2024
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

No branches or pull requests

3 participants