Skip to content

Migration to ESM#515

Open
gfontorbe wants to merge 8 commits intomasterfrom
gfontorbe/esm-migration
Open

Migration to ESM#515
gfontorbe wants to merge 8 commits intomasterfrom
gfontorbe/esm-migration

Conversation

@gfontorbe
Copy link
Contributor

Migrates the packages to ESM for Sprotty 2.0.0

Upgrading dependencies will come in a future PR

Copy link
Contributor

@kaisalmen kaisalmen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good already. I have some suggestions. I did not check each and every import change and trust that VS Code's import clean-up and re-order does its job corrrectly.

We could have a direct chat tomorrow. 🙂

@@ -16,4 +16,4 @@

// By default we export the Inversify-ready version. If you want to use the plain
// version, import from `sprotty-elk/lib/elk-layout`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that is still true, then you need to define the specific sub-export in the package.json.

Copy link
Contributor

@spoenemann spoenemann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for this effort @gfontorbe!

Some of the examples are currently broken: Micro-Layout Showcase, Class Diagram, Random Graph, SVG

@gfontorbe
Copy link
Contributor Author

Thanks a lot for this effort @gfontorbe!

Some of the examples are currently broken: Micro-Layout Showcase, Class Diagram, Random Graph, SVG

@spoenemann this was due to how the ES standard deals with classes optional fields. ES2017 was not emitting the optional field while ES2022 is emitting optionalField=undefined.

Our check for isLayoutContainer was only checking if there was a property 'layout' in the object, but never if the value was undefined.

This is now fixed.

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.

3 participants