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

Turn into ES module #870

Draft
wants to merge 11 commits into
base: esm
Choose a base branch
from
Draft

Turn into ES module #870

wants to merge 11 commits into from

Conversation

nikku
Copy link
Member

@nikku nikku commented Mar 12, 2024

This bumps dependencies to ES modules.

This turns diagram-js to be an ES module.

Depends on #869
Closes #863

@bpmn-io-tasks bpmn-io-tasks bot added the in progress Currently worked on label Mar 12, 2024
@nikku
Copy link
Member Author

nikku commented Mar 13, 2024

This PR poses the question: How do we want downstream libraries consume diagram-js?

In bpmn-js and other libraries we currently consume like this:

import ModelUtil from 'diagram-js/lib/ModelUtil';
import zoomScroll from 'diagram-js/lib/navigation/zoomscroll';

Also possible right now, explicit import:

import ModelUtil from 'diagram-js/lib/ModelUtil.js';
import zoomScroll from 'diagram-js/lib/navigation/zoomscroll/index.js';

As we create ES modules with an exports declaration we have to be explicit about what we export (and hence can be imported).


Proposed in this PR we force our users to switch to explicit imports, which is a major breaking change for all consumers (cf. bpmn-js test ⬇️):

ERROR in ./test/util/custom-rules/CustomRules.js 3:0-70
Module not found: Error: Package path ./lib/features/rules/RuleProvider is not exported from package [PRJ]/bpmn-js/node_modules/diagram-js (see exports field in [PRJ]/bpmn-js/node_modules/diagram-js/package.json)
 @ ./test/util/custom-rules/index.js 1:0-40 5:25-36
 @ ./test/spec/features/context-pad/ContextPadProviderSpec.js 27:0-59 39:4-21
 @ ./test/ sync (spec%7Cintegration).*Spec\.js$ ./spec/features/context-pad/ContextPadProviderSpec.js test/spec/features/context-pad/ContextPadProviderSpec.js
 @ ./test/testBundle.js 1:15-74

webpack 5.88.1 compiled with 273 errors and 39 warnings in 1670 ms

So we have three options:

  • Build exports in a way that current import scenario works
  • Switch to semantic ESM (import via file extension)
  • Do not define exports field

@barmac
Copy link
Member

barmac commented Mar 19, 2024

If we are to ship this, we could consider writing a codemod to make the migration easier (for example with https://github.com/facebook/jscodeshift).

@nikku nikku removed their assignment Oct 17, 2024
@nikku nikku added backlog Queued in backlog and removed in progress Currently worked on labels Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Queued in backlog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants