cytoscape / cytoscape.js-dagre

The Dagre layout for DAGs and trees for Cytoscape.js
MIT License
245 stars 82 forks source link

Converting this to ESM #120

Closed ayjayt closed 4 months ago

ayjayt commented 6 months ago

Hello! I'm about to start converting this to ESM (hopefully itll be as easy as the dependencies).

I have done the dependencies:

graphlib

dagre

However, if the author doesn't pull and release them, then the official npm repos wont support ESM here either!

I want it and need it tho so I'll be using a fork if I have to.

Pinging @maxkfranz for fun!

ayjayt commented 6 months ago

Not packing dagre in w/ cytoscape-dagre.js seems a bit offbeat to me if using webpack.

maxkfranz commented 6 months ago

Should coordinate with https://github.com/cytoscape/cytoscape.js-automove/pull/34

stale[bot] commented 6 months ago

This issue has been automatically marked as stale, because it has not had activity within the past 14 days. It will be closed if no further activity occurs within the next 7 days. If a feature request is important to you, please consider making a pull request. Thank you for your contributions.

aloisklink commented 6 months ago

FYI, there's an ESM fork of dagre at https://www.npmjs.com/package/dagre-d3-es.

It's not a one-to-one comparison though, since that fork combines:

Although, beware of the download count, I know it has over ~500k weekly downloads, but almost all of them are just from one project, so it it's not as well tested as the official dagre library.

stale[bot] commented 5 months ago

This issue has been automatically marked as stale, because it has not had activity within the past 14 days. It will be closed if no further activity occurs within the next 7 days. If a feature request is important to you, please consider making a pull request. Thank you for your contributions.

ayjayt commented 5 months ago

@aloisklink I think they just merged my changes to the main library! So the main library should be native esm now. I'll be reviewing all this this weekend to see where we're at!

edit: it looks like they reverted them, am investigating

ayjayt commented 5 months ago

Yeah so, honestly, it seems kinda weird over there.

I think my conversions might be a hard pill to swallow, rollup provides a better experience.

But still, they reverted a dependency and not a defendant but I don't see how that builds, so. This project is on my backburner, but maybe I'll get back to it before anyone else does.

stale[bot] commented 5 months ago

This issue has been automatically marked as stale, because it has not had activity within the past 14 days. It will be closed if no further activity occurs within the next 7 days. If a feature request is important to you, please consider making a pull request. Thank you for your contributions.