Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the @joint/core ESM surface to use namespace-style exports (via export * as ...) and simplifies wrapper entrypoints accordingly, while adjusting a Node.js sanity test in @joint/layout-directed-graph.
Changes:
- Convert many
@joint/coreexports to namespace exports and simplifyjoint.mjs/ wrapper re-exports. - Extract
setThemeinto its own module and re-export it fromcore.mjs. - Update the Node.js test assertions for the directed-graph package.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/joint-layout-directed-graph/test/nodejs/nodejs.js | Updates assertions around what @joint/core exposes in Node.js tests. |
| packages/joint-core/wrappers/joint.wrapper.mjs | Simplifies wrapper exports by re-exporting everything from src/core.mjs and namespace-exporting shapes. |
| packages/joint-core/src/shapes/index.mjs | Switches to namespace export for standard shapes. |
| packages/joint-core/src/setTheme.mjs | New module containing the extracted setTheme implementation. |
| packages/joint-core/src/layout/index.mjs | Switches Port / PortLabel to namespace exports. |
| packages/joint-core/src/g/index.mjs | Switches intersection to a namespace export. |
| packages/joint-core/src/core.mjs | Central change: converts exports to namespace exports and re-exports setTheme. |
| packages/joint-core/joint.mjs | Simplifies public ESM entrypoint by re-exporting everything from src/core.mjs plus shapes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -21,8 +22,9 @@ describe('require jointjs', function() { | |||
|
|
|||
| it('should not contain layout.DirectedGraph object', function() { | |||
|
|
|||
| joint.should.have.property('layout'); | |||
| joint.layout.should.not.have.property('DirectedGraph'); | |||
| should.exist(joint); | |||
| should.exist(joint.layout, 'joint.layout is undefined'); | |||
| should.not.exist(joint.layout.DirectedGraph, 'joint.layout.DirectedGraph should not exist'); | |||
There was a problem hiding this comment.
should is referenced as a free variable (should.exist(...), should.not.exist(...)) but the file only calls require('should'); without binding the return value. In strict mode this relies on should being installed as a global by the library, which is brittle and can fail if the behavior/version changes. Prefer assigning it explicitly (e.g., bind the require result) or keep using the chained ...should... assertions consistently.
| export { V }; | ||
| export const Vectorizer = V; | ||
| export const layout = { PortLabel, Port }; | ||
| export * as layout from './layout/index.mjs'; | ||
| export { env } from './env/index.mjs'; |
There was a problem hiding this comment.
Changing layout from a plain object ({ PortLabel, Port }) to a re-exported module namespace (export * as layout ...) makes joint.layout a module namespace object in ESM consumers, which is non-extensible/immutable by spec. If any consumers/plugins rely on attaching additional properties under joint.layout at runtime (e.g., joint.layout.DirectedGraph = ...), this will break. If runtime extension is still intended/supported for the ESM entrypoint, consider exporting layout as a plain object built from the layout module instead of a namespace export.
Description
Clearly and concisely describe your changes in detail.
Motivation and Context
Why is this change required? What problem does it solve? If applicable, describe the new features the solution brings.
Screenshots (if appropriate):