feat: add CommonJS support to npm package #826
Conversation
The new commonjs resources are built with a new tsc command and now referenced in the package.json file. Add new examples using CommonJS with Node.js to demonstrate the new support: - TypeScript example: show the integration with jest and ts-jest - JavaScript example: use `maxGraph` in a headless environment with Node.js (and jsdom). This example exports Graph Model as XML, and exports the rendered Graph to SVG. In addition, add missing git extension in the repository attribute of the package.json file.
WalkthroughThis change introduces dual ESM and CommonJS (CJS) build support for the Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Jest as Jest (ts-jest)
participant Node as Node.js (jsdom)
participant MaxCore as @maxgraph/core (ESM/CJS)
participant Graph as Graph Instance
Dev->>Jest: Run tests (CommonJS)
Jest->>MaxCore: require('@maxgraph/core') (CJS)
MaxCore-->>Jest: Provide CJS API
Jest->>Graph: Instantiate BaseGraph, run tests
Dev->>Node: Run headless script (CommonJS)
Node->>MaxCore: require('@maxgraph/core') (CJS)
MaxCore-->>Node: Provide CJS API
Node->>Graph: Instantiate BaseGraph, export XML/SVG
Assessment against linked issues
Possibly related PRs
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)packages/js-example-nodejs/src/index.mjs (1)🪛 ESLintpackages/js-example-nodejs/src/index.mjs[error] 21-21: Unexpected console statement. (no-console) 🔇 Additional comments (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
This is more portable, the previous implementation failed on Windows.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (15)
packages/core/tsconfig.json (1)
9-9: Redirect ESM build output: ChangingoutDirto./lib/esmcorrectly isolates the ESM artifacts for the dual-package setup. For improved build hygiene, consider configuring a distincttsBuildInfoFileor using separate project references/tsconfig files to avoid incremental build caches clashing between the ESM and CJS outputs.packages/ts-example-jest-commonjs/README.md (1)
9-14: Augment setup with test execution instructions: After buildingmaxGraph@core, include the command to run the Jest tests (e.g.,npm testornpx jest). This completes the end-to-end setup guidance and helps users verify their environment.README.md (1)
156-156: Consistent link formatting: Thejs-example-nodejslink uses./packages/js-example-nodejs, while other entries usepackages/...without./. Consider removing the./prefix for consistency.packages/ts-example-jest-commonjs/package.json (1)
1-17: LGTM: Package configuration appropriately set up for CommonJS example with JestThe package configuration is properly set up with the correct dependencies for Jest testing in a TypeScript CommonJS environment.
Consider adding a description field to provide context about this example package's purpose, and specify its relationship to the core package (potentially as a peer dependency or installation instruction in a README).
packages/js-example-nodejs/src/index.mjs (2)
2-2: Update copyright yearThe copyright year is set to 2025, which is in the future.
-Copyright 2025-present The maxGraph project Contributors +Copyright 2024-present The maxGraph project Contributors
17-18: Clarify TODO comment with issue referenceThe TODO comment mentions "XXX" which appears to be a placeholder for an issue reference.
Consider updating the TODO with a specific issue number or more descriptive context about the planned fix:
-// TODO this fails because the import statements in the files of the maxGraph package doesn't add the "js" file extension -// Node.js requires such extensions. This should be fixed when implementing XXX +// TODO this fails because the import statements in the files of the maxGraph package don't include the ".js" file extension +// Node.js requires these extensions for ESM imports. This will be fixed when implementing issue #XXX (ESM path resolution)packages/js-example-nodejs/README.md (2)
1-1: Missing article in titleConsider adding the article "a" before "headless" for improved readability.
-# Integrate `maxGraph` in headless environment with Node.js +# Integrate `maxGraph` in a headless environment with Node.js🧰 Tools
🪛 LanguageTool
[uncategorized] ~1-~1: You might be missing the article “a” here.
Context: # IntegratemaxGraphin headless environment with Node.js Demonstrate h...(AI_EN_LECTOR_MISSING_DETERMINER_A)
15-22: Consider adding example execution instructionsThe README provides good setup instructions but doesn't include how to run the examples after setup.
Consider adding a "Running the examples" section with commands to execute both the CommonJS and ESM examples:
Build maxgraph@core > From the `packages/core` directory, run `npm run build`. + +## Running the examples + +Run the CommonJS example: +> From the `packages/js-example-nodejs` directory, run `npm run example:cjs`. + +Run the ESM example (currently not working): +> From the `packages/js-example-nodejs` directory, run `npm run example:esm`.packages/ts-example-jest-commonjs/__tests__/index.ts (1)
1-15: Update copyright yearThe copyright year is set to 2025, which is in the future. Consider updating to the current year.
-Copyright 2025-present The maxGraph project Contributors +Copyright 2023-present The maxGraph project ContributorsEnsure you use the correct starting year for this new file.
packages/core/scripts/generate-cjs-package-json.mjs (3)
1-15: Update copyright yearThe copyright year is set to 2025, which is in the future. Consider updating to the current year.
-Copyright 2025-present The maxGraph project Contributors +Copyright 2023-present The maxGraph project ContributorsEnsure you use the correct starting year for this new file.
19-21: Add error handling for file operationsThe
writeJsonContentToFilefunction doesn't include error handling, which could lead to silent failures.const writeJsonContentToFile = (path, obj) => { - writeFileSync(path, JSON.stringify(obj, null, 2) + '\n'); + try { + writeFileSync(path, JSON.stringify(obj, null, 2) + '\n'); + } catch (error) { + console.error(`Failed to write package.json to ${path}:`, error); + process.exit(1); + } };
23-29: Consider handling directory creationThe script assumes the
lib/cjsdirectory exists, but it might not if the TypeScript compilation failed or hasn't been run yet.+import { existsSync, mkdirSync } from 'node:fs'; +import { dirname } from 'node:path'; console.info('Writing package.json file for CommonJS...'); const path = 'lib/cjs/package.json'; + +// Ensure the directory exists +const dir = dirname(path); +if (!existsSync(dir)) { + console.info(`Creating directory ${dir}...`); + mkdirSync(dir, { recursive: true }); +} + const packageJson = { type: 'commonjs' }; writeJsonContentToFile(path, packageJson); console.info('package.json file for CommonJS written to', path);🧰 Tools
🪛 ESLint
[error] 23-23: Unexpected console statement.
(no-console)
[error] 29-29: Unexpected console statement.
(no-console)
packages/js-example-nodejs/src/index.cjs (2)
1-15: Update copyright yearThe copyright year is set to 2025, which is in the future. Consider updating to the current year.
-Copyright 2025-present The maxGraph project Contributors +Copyright 2023-present The maxGraph project ContributorsEnsure you use the correct starting year for this new file.
84-101: Consider adding JSDoc parameters and return typeThe
exportmethod could benefit from complete JSDoc documentation of parameters and return type./** + * Exports the graph as an SVG string + * @param {Object} [options] - Export options + * @param {number} [options.margin=10] - Margin to add around the graph bounds + * @returns {string} The SVG content as a string + */ export(options) {packages/core/package.json (1)
56-58: Simplify and order build scripts
The current wildcardrun-s build:*will runbuild:cjsthenbuild:esmalphabetically, which may be non-intuitive. Also, loggingtsc --versionon every build can clutter CI logs. Consider explicitly sequencing the builds and removing the version check:- "build": "run-s build:*", - "build:esm": "tsc --version && tsc", - "build:cjs": "tsc --version && tsc --outDir lib/cjs --module commonjs && node scripts/generate-cjs-package-json.mjs", + "build": "run-s build:esm build:cjs", + "build:esm": "tsc", + "build:cjs": "tsc --outDir lib/cjs --module commonjs && node scripts/generate-cjs-package-json.mjs",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
.github/dependabot.yml(1 hunks)README.md(1 hunks)packages/core/package.json(2 hunks)packages/core/scripts/generate-cjs-package-json.mjs(1 hunks)packages/core/tsconfig.json(1 hunks)packages/js-example-nodejs/README.md(1 hunks)packages/js-example-nodejs/package.json(1 hunks)packages/js-example-nodejs/src/index.cjs(1 hunks)packages/js-example-nodejs/src/index.mjs(1 hunks)packages/ts-example-jest-commonjs/README.md(1 hunks)packages/ts-example-jest-commonjs/__tests__/index.ts(1 hunks)packages/ts-example-jest-commonjs/jest.config.cjs(1 hunks)packages/ts-example-jest-commonjs/package.json(1 hunks)packages/website/docs/demo-and-examples.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
packages/website/docs/demo-and-examples.md (3)
Learnt from: tbouffard
PR: maxGraph/maxGraph#791
File: packages/ts-example/vite.config.js:30-30
Timestamp: 2025-04-29T13:25:31.494Z
Learning: In the maxGraph project, each example package (ts-example, ts-example-selected-features, ts-example-without-defaults) implements different use cases with varying features, resulting in different application sizes. Therefore, each package has its own specific chunkSizeWarningLimit value in its vite.config.js file, calibrated to its expected bundle size.
Learnt from: tbouffard
PR: maxGraph/maxGraph#774
File: packages/ts-example-selected-features/package.json:11-13
Timestamp: 2025-04-22T16:34:40.309Z
Learning: In the maxGraph project, TypeScript is defined as a dependency in the root package.json and not in individual package.json files for example packages. The build scripts in example packages can use `tsc` because it's available through this hoisted dependency in the monorepo structure.
Learnt from: tbouffard
PR: maxGraph/maxGraph#774
File: packages/ts-example-selected-features/package.json:11-13
Timestamp: 2025-04-22T16:34:40.309Z
Learning: In the maxGraph project, TypeScript is defined as a dependency in the root package.json (~5.8.2) and not in individual package.json files for example packages. The build scripts in example packages can use `tsc` commands because the TypeScript dependency is hoisted in the monorepo structure.
README.md (3)
Learnt from: tbouffard
PR: maxGraph/maxGraph#774
File: packages/ts-example-selected-features/package.json:11-13
Timestamp: 2025-04-22T16:34:40.309Z
Learning: In the maxGraph project, TypeScript is defined as a dependency in the root package.json (~5.8.2) and not in individual package.json files for example packages. The build scripts in example packages can use `tsc` commands because the TypeScript dependency is hoisted in the monorepo structure.
Learnt from: tbouffard
PR: maxGraph/maxGraph#774
File: packages/ts-example-selected-features/package.json:11-13
Timestamp: 2025-04-22T16:34:40.309Z
Learning: In the maxGraph project, TypeScript is defined as a dependency in the root package.json and not in individual package.json files for example packages. The build scripts in example packages can use `tsc` because it's available through this hoisted dependency in the monorepo structure.
Learnt from: tbouffard
PR: maxGraph/maxGraph#791
File: packages/ts-example/vite.config.js:30-30
Timestamp: 2025-04-29T13:25:31.494Z
Learning: In the maxGraph project, each example package (ts-example, ts-example-selected-features, ts-example-without-defaults) implements different use cases with varying features, resulting in different application sizes. Therefore, each package has its own specific chunkSizeWarningLimit value in its vite.config.js file, calibrated to its expected bundle size.
🧬 Code Graph Analysis (1)
packages/ts-example-jest-commonjs/__tests__/index.ts (1)
packages/core/src/types.ts (1)
CellStyle(50-75)
🪛 ESLint
packages/core/scripts/generate-cjs-package-json.mjs
[error] 23-23: Unexpected console statement.
(no-console)
[error] 29-29: Unexpected console statement.
(no-console)
packages/js-example-nodejs/src/index.mjs
[error] 21-21: Unexpected console statement.
(no-console)
packages/ts-example-jest-commonjs/jest.config.cjs
[error] 18-18: 'module' is not defined.
(no-undef)
[error] 21-21: Unnecessary escape character: ..
(no-useless-escape)
packages/js-example-nodejs/src/index.cjs
[error] 17-17: A require() style import is forbidden.
(@typescript-eslint/no-require-imports)
[error] 17-17: 'require' is not defined.
(no-undef)
[error] 18-18: A require() style import is forbidden.
(@typescript-eslint/no-require-imports)
[error] 18-18: 'require' is not defined.
(no-undef)
[error] 19-19: A require() style import is forbidden.
(@typescript-eslint/no-require-imports)
[error] 19-19: 'require' is not defined.
(no-undef)
[error] 21-21: Unexpected console statement.
(no-console)
[error] 59-59: Unexpected console statement.
(no-console)
[error] 71-71: Unexpected console statement.
(no-console)
[error] 104-104: Unexpected console statement.
(no-console)
[error] 107-107: Unexpected console statement.
(no-console)
🪛 LanguageTool
packages/js-example-nodejs/README.md
[uncategorized] ~1-~1: You might be missing the article “a” here.
Context: # Integrate maxGraph in headless environment with Node.js Demonstrate h...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
🪛 markdownlint-cli2 (0.17.2)
packages/js-example-nodejs/README.md
13-13: No empty links
null
(MD042, no-empty-links)
🔇 Additional comments (15)
.github/dependabot.yml (1)
55-61: Expand test dependency monitoring: Adding@types/jest,jsdom,jsdom-*, andts-jestto the test group ensures that the new example projects' dependencies remain up-to-date and secure.packages/ts-example-jest-commonjs/README.md (1)
1-6: Clear introduction and context: The title and overview succinctly explain the goal of runningmaxGraphin CommonJS Jest tests with ts-jest andjest-jsdom. This sets the right expectations for users.README.md (1)
152-152: Include new TypeScript example: Addingts-example-jest-commonjsto the examples list keeps the documentation in sync with the new CommonJS Jest testing setup.packages/website/docs/demo-and-examples.md (2)
29-29: Document TypeScript Jest example: Includingts-example-jest-commonjsunder the TypeScript Examples section accurately guides users to the new Jest+ts-jest CommonJS setup.
35-35: Document headless Node.js example: Addingjs-example-nodejsto the JavaScript Examples ensures users know how to runmaxGraphin a headless Node.js environment with jsdom.packages/js-example-nodejs/package.json (1)
6-10: Build script only runs CommonJS exampleThe
buildscript only runsbuild:cjsand not the ESM example, which aligns with the ESM implementation being incomplete as mentioned in the AI summary.packages/js-example-nodejs/src/index.mjs (1)
21-21: Console statement in example codeThe console statement is flagged by ESLint, but is appropriate for an example showing how the code works.
🧰 Tools
🪛 ESLint
[error] 21-21: Unexpected console statement.
(no-console)
packages/ts-example-jest-commonjs/__tests__/index.ts (2)
21-24: Good test for BaseGraph.isOrthogonal functionalityThis test correctly demonstrates using the maxGraph API with Jest in a CommonJS environment.
26-36: Good test for cell style managementThis test effectively demonstrates modifying cell styles and verifying both the cell's style property and the view state. Great example for showing style management in maxGraph.
packages/js-example-nodejs/src/index.cjs (2)
64-69: Good practice: Creating output directoryThe code correctly checks if the output directory exists and creates it if necessary, ensuring the file operations won't fail.
25-51: Well-structured graph creation exampleThe batch update pattern is correctly demonstrated, showing how to insert multiple graph elements efficiently. This is a good example for users learning the maxGraph API.
packages/core/package.json (4)
24-25: Verify ESM export mapping
Theexports.importblock correctly provides separate paths for TypeScript and runtime consumers. This aligns with the"module"and"types"fields above.
27-29: Verify CJS export mapping and type declarations
Theexports.requireblock directsrequire()tolib/cjs/index.jsand its types tolib/cjs/index.d.ts. Confirm thatgenerate-cjs-package-json.mjsnot only writes apackage.jsonwith"type": "commonjs"intolib/cjs, but also that a.d.tslives atlib/cjs/index.d.ts.You can adapt the above shell snippet to check for
lib/cjs/index.d.ts.
48-48: Fix repository URL to include.gitsuffix
This corrects the missing extension in therepository.urlfield.
18-20:Details
❓ Verification inconclusive
Add dual entry points for CJS and ESM
The new"main","module", and"types"fields correctly point to the CJS and ESM bundles. Ensure that after running the build scripts, the fileslib/cjs/index.js,lib/esm/index.js, andlib/esm/index.d.tsactually exist and match these paths.Run a quick check after building:
🏁 Script executed:
#!/bin/bash # Verify outputs for CJS and ESM entry points for f in lib/cjs/index.js lib/esm/index.js lib/esm/index.d.ts; do if [ ! -f "$f" ]; then echo "Missing build artifact: $f" exit 1 fi done echo "All entry points are present."Length of output: 197
🏁 Script executed:
#!/bin/bash # List all build artifacts under lib to verify presence of entry points find lib -type fLength of output: 57
Verify build output for CJS and ESM entry points
Themain,module, andtypesfields inpackages/core/package.jsonpoint to:
lib/cjs/index.jslib/esm/index.jslib/esm/index.d.tsHowever, no
lib/directory was found in the PR’s build environment. Please run the package’s build locally (e.g.npm run build) and confirm that those files are generated and match the declared entry points. You can use this quick check:#!/bin/bash npm run build for f in lib/cjs/index.js lib/esm/index.js lib/esm/index.d.ts; do if [ ! -f "$f" ]; then echo "Missing build artifact: $f" exit 1 fi done echo "All entry points are present."Once verified, update the PR (or CI) accordingly.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/js-example-nodejs/src/index.cjs (1)
94-96: Enhance SimpleSvgExporter class with error handlingThe exporter class doesn't include any error handling, which could lead to hard-to-debug issues. Consider adding basic error checking.
// we are sure that we have an SVG node here, it is always created at Graph creation -const svgNode = this.graph.container.querySelector('svg'); +const svgNode = this.graph.container.querySelector('svg'); +if (!svgNode) { + throw new Error('SVG element not found in graph container'); +}
🧹 Nitpick comments (1)
packages/ts-example-jest-commonjs/jest.config.cjs (1)
18-18: Ignore ESLint 'module is not defined' warning.The ESLint warning about 'module' not being defined is a false positive for CommonJS files. In
.cjsfiles, 'module' is automatically defined in the global scope by Node.js.If you want to silence this warning in the future, you could add a comment directive:
/** @type {import('ts-jest').JestConfigWithTsJest} **/ +/* global module */ module.exports = {🧰 Tools
🪛 ESLint
[error] 18-18: 'module' is not defined.
(no-undef)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/js-example-nodejs/src/index.cjs(1 hunks)packages/ts-example-jest-commonjs/jest.config.cjs(1 hunks)
🧰 Additional context used
🪛 ESLint
packages/js-example-nodejs/src/index.cjs
[error] 17-17: A require() style import is forbidden.
(@typescript-eslint/no-require-imports)
[error] 17-17: 'require' is not defined.
(no-undef)
[error] 18-18: A require() style import is forbidden.
(@typescript-eslint/no-require-imports)
[error] 18-18: 'require' is not defined.
(no-undef)
[error] 19-19: A require() style import is forbidden.
(@typescript-eslint/no-require-imports)
[error] 19-19: 'require' is not defined.
(no-undef)
[error] 21-21: Unexpected console statement.
(no-console)
[error] 59-59: Unexpected console statement.
(no-console)
[error] 71-71: Unexpected console statement.
(no-console)
[error] 105-105: Unexpected console statement.
(no-console)
[error] 108-108: Unexpected console statement.
(no-console)
packages/ts-example-jest-commonjs/jest.config.cjs
[error] 18-18: 'module' is not defined.
(no-undef)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build (windows-2022)
- GitHub Check: build (ubuntu-22.04)
🔇 Additional comments (5)
packages/ts-example-jest-commonjs/jest.config.cjs (1)
1-23: Well-structured Jest configuration for CommonJS support.The configuration is correctly set up with:
- Proper JSDoc type annotation for TypeScript intellisense
- JSDOM test environment (necessary for maxGraph's browser dependencies)
- Correctly configured TypeScript transformer
The past issue with the regex escape character has been properly addressed with the correct double backslash pattern.
🧰 Tools
🪛 ESLint
[error] 18-18: 'module' is not defined.
(no-undef)
packages/js-example-nodejs/src/index.cjs (4)
1-20: Well-implemented CommonJS file header and importsThe file structure correctly follows CommonJS patterns with proper license header and required imports. The use of
jsdom-globalis appropriate for creating a DOM environment in Node.js.Note: The ESLint errors about
require()style imports are expected for a CommonJS file and can be safely ignored.🧰 Tools
🪛 ESLint
[error] 17-17: A
require()style import is forbidden.(@typescript-eslint/no-require-imports)
[error] 17-17: 'require' is not defined.
(no-undef)
[error] 18-18: A
require()style import is forbidden.(@typescript-eslint/no-require-imports)
[error] 18-18: 'require' is not defined.
(no-undef)
[error] 19-19: A
require()style import is forbidden.(@typescript-eslint/no-require-imports)
[error] 19-19: 'require' is not defined.
(no-undef)
25-51: Good use of batch update patternThe graph creation code uses
batchUpdateto efficiently add multiple elements at once, which is a good practice for performance. The vertex and edge configurations are clearly defined with appropriate styling options.
73-103: Well-documented SVG exporter implementation with clear limitationsThe
SimpleSvgExporterclass is well-documented with JSDoc comments explaining its limitations regarding HTML labels. The implementation correctly calculates graph bounds with margins and properly constructs the SVG.
17-19:Details
❓ Verification inconclusive
Verify CommonJS compatibility with maxGraph core
This example demonstrates using maxGraph with CommonJS imports, but it would be beneficial to verify that all required features work correctly with this import style.
🏁 Script executed:
#!/bin/bash # Check if all required maxGraph features are exported in the CommonJS build # Look at the package.json to see how the CommonJS exports are configured rg -A 10 "exports" package.json # Check if there are any ESM-only features by looking at type declarations fd -e d.ts -x grep -l "export" {} \; | xargs grep -l "export" | head -n 5Length of output: 106
Verify CommonJS exports for @maxgraph/core
This example uses CommonJS (
require('@maxgraph/core')) to pull inconstants,Graph,ModelXmlSerializer, andRectangle. Please confirm that all of these are actually exposed by the CommonJS build:
Check the root
package.jsonfor a"type": "module"field (files with a.cjsextension are exempt, but good to verify).Inspect
node_modules/@maxgraph/core/package.jsonand its"main"or"exports"to ensure they point at a CommonJS entry that includes the above symbols.Run a quick smoke test:
node -e "const mod = require('@maxgraph/core'); console.log(Object.keys(mod));"Verify that
constants,Graph,ModelXmlSerializer, andRectangleappear in the output without errors.🧰 Tools
🪛 ESLint
[error] 17-17: A
require()style import is forbidden.(@typescript-eslint/no-require-imports)
[error] 17-17: 'require' is not defined.
(no-undef)
[error] 18-18: A
require()style import is forbidden.(@typescript-eslint/no-require-imports)
[error] 18-18: 'require' is not defined.
(no-undef)
[error] 19-19: A
require()style import is forbidden.(@typescript-eslint/no-require-imports)
[error] 19-19: 'require' is not defined.
(no-undef)
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/js-example-nodejs/README.md (1)
23-23:⚠️ Potential issueRemove stray line artifact
The trailing “23” appears to be an unintended artifact—please remove it to avoid confusion.
♻️ Duplicate comments (1)
packages/js-example-nodejs/README.md (1)
13-13: Empty placeholder link needs to be filled
Replace[xxxx]()with the actual issue or PR number that will implement ESM compatibility for Node.js, for example:This will be implemented in the future with [issue #824](https://github.com/maxGraph/maxGraph/issues/824).🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
13-13: No empty links
null(MD042, no-empty-links)
🧹 Nitpick comments (3)
packages/js-example-nodejs/README.md (3)
8-11: Enhance example section with usage details and file references
The README mentions CommonJS and ESM examples but doesn’t link to or show how to execute them. Consider adding direct links tosrc/index.cjsandsrc/index.mjs, and commands (e.g.,node src/index.cjs) so users can run the demos immediately.
12-12: Correct grammar in ESM compatibility note
Change “doesn't provide ESM compatible yet with Node.js” to “doesn't yet provide ESM compatibility for Node.js” for clarity and grammatical accuracy.
16-22: Use fenced code blocks for setup commands
Switch from blockquotes to fenced code blocks so commands are more readable and easily copy-pasted. For example:- > From the repository root, run `npm install`. + ```bash + npm install + ``` - > From the `packages/core` directory, run `npm run build`. + ```bash + cd packages/core + npm run build + ```
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md(1 hunks)packages/core/src/view/GraphView.ts(22 hunks)packages/js-example-nodejs/README.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
🧰 Additional context used
🧠 Learnings (1)
packages/js-example-nodejs/README.md (1)
Learnt from: tbouffard
PR: maxGraph/maxGraph#826
File: packages/js-example-nodejs/src/index.cjs:64-69
Timestamp: 2025-05-13T12:54:55.196Z
Learning: For example code in the maxGraph repository, maintainers prefer to keep scripts simple without error handling to focus on demonstrating core functionality, especially in demonstration scripts like those in packages/js-example-nodejs.
🪛 markdownlint-cli2 (0.17.2)
packages/js-example-nodejs/README.md
13-13: No empty links
null
(MD042, no-empty-links)
🔇 Additional comments (23)
packages/core/src/view/GraphView.ts (23)
317-318: Documentation updated correctlyThe comment now accurately describes the DOM node returned by
getCanvas()method.
353-356: Documentation type references updated appropriatelyThe JSDoc comments have been updated to use modern type references (
Cellinstead of outdatedmxCell), aligning with the module's type system.
383-384: Documentation type reference updated correctlyUpdated type reference from outdated
mxCellto currentCelltype.
537-539: Documentation clarification improvedThe comment for
validate()method has been updated with proper type references.
620-621: Type reference updated in parameter documentationUpdated from
mxRectangletoRectanglefor consistency with the codebase's type system.
688-690: Comment clarification improvedThe comment now better explains the purpose of the event listeners for graph event dispatching.
762-764: Parameter type references updatedUpdated from
mxImageShapetoImageShapeandmxImagetoImageto align with current type system.
784-787: Documentation type reference updated appropriatelyUpdated from
mxCelltoCellfor thevalidateCellmethod parameter.
811-814: Documentation type reference updatedParameter documentation now correctly uses
Cellinstead of outdatedmxCell.
849-851: Comment clarity improvedThe comment now more accurately describes the handling of changes to vertex paint bounds.
1130-1132: Parameter documentation updated with current type referencesUpdated from
mxCellStatetoCellStatefor method parameters.
1150-1152: Documentation references updated for parametersUpdated from
mxCellStatetoCellStatefor consistent type naming throughout the codebase.
1187-1188: Parameter type documentation updatedUpdated from
mxCellStatetoCellStatefor theupdateBoundsFromStencilmethod.
1640-1643: Documentation type reference updatedChanged from
mxCelltoCellfor thegetVisibleTerminalmethod parameter.
1671-1676: Documentation updated with correct referencesThe method documentation now uses proper type references and explains the purpose clearly.
1738-1739: Type reference in parameter documentation updatedUpdated from
mxGeometrytoGeometryfor consistency with current type system.
1800-1803: Documentation reference updated for method parameterUpdated from
mxCellStatetoCellStatefor thegetRelativePointmethod.
1893-1900: Documentation clarity improvedThe comment for
updateEdgeLabelOffsetnow more accurately describes the purpose of the method.
1946-1949: Parameter type reference updatedChanged from
mxCelltoCellfor thegetStatemethod parameter.
1966-1970: Method documentation updated with correct type referenceUpdated from
mxCelltoCellfor array parameters ingetCellStates.
1991-1992: Parameter type reference updatedChanged from
mxCelltoCellin theremoveStatemethod documentation.
2009-2010: Documentation reference updatedChanged from
mxCelltoCellfor thecreateStatemethod parameter.
1-2395: Overall documentation improvements are well-executedThe changes in this file consist solely of documentation updates that replace outdated type references in JSDoc comments (e.g.,
mxCelltoCell,mxCellStatetoCellState,mxRectangletoRectangle, etc.) and improve comment clarity. These changes align perfectly with the PR's goal of modernizing the codebase to support both ESM and CommonJS module formats.The consistent replacement of legacy
mxprefixed types with their modern equivalents ensures that the documentation matches the actual types used in the codebase, improving developer experience and code maintainability.🧰 Tools
🪛 Biome (1.9.4)
[error] 584-584: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 589-589: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 699-699: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1193-1195: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 2189-2189: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 2363-2363: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/js-example-nodejs/README.md (1)
13-13:⚠️ Potential issueFill in the placeholder link
The placeholder link
[xxxx]()needs to reference the actual issue tracking ESM compatibility (e.g. issue #824). Please replace it accordingly:-This will be implemented in the future with [xxxx](). +This will be implemented in the future with [issue #824](https://github.com/maxGraph/maxGraph/issues/824).🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
13-13: No empty links
null(MD042, no-empty-links)
🧹 Nitpick comments (4)
packages/js-example-nodejs/README.md (4)
1-2: Refine Heading for ClarityConsider making the top-level heading more descriptive by referencing the actual package name (
@maxgraph/core) and highlighting that this is the CommonJS Node.js example. For example:-# Integrate `maxGraph` in a headless environment with Node.js +# Node.js Headless CommonJS Example for `@maxgraph/core`
3-7: Consistent Code FormattingWrap library names in backticks for consistency and clarity. For example:
-Demonstrate how to integrate maxGraph with Node.js to use it in a headless environment. +Demonstrate how to integrate `@maxgraph/core` with Node.js to use it in a headless environment. -In this example, `jsdom` is used to provide an implementation of the browser objects whose `maxGraph` depends on. +In this example, `jsdom` is used to provide implementations of the browser objects on which `@maxgraph/core` depends.
8-12: Include a Quick Usage SnippetTo make this README more actionable, it would be helpful to include a minimal CommonJS code example showing how to import and instantiate a graph. For instance:
```js // packages/js-example-nodejs/src/index.cjs const { mxGraph, mxCodec, mxUtils } = require('@maxgraph/core'); const jsdom = require('jsdom'); const { JSDOM } = jsdom; // setup jsdom, create graph, export to XML/SVG...--- `16-23`: **Verify CJS Build Instructions & Improve Command Formatting** 1. Ensure the setup instructions cover the new CommonJS build step (e.g., `npm run build:cjs` if applicable). 2. Consider switching from blockquotes (`>`) to fenced code blocks for readability: ```diff -> From the repository root, run `npm install`. +```bash +npm install +``` -> From the `packages/core` directory, run `npm run build`. +```bash +cd packages/core +npm run build # builds ESM & CJS outputs (or specify `npm run build:cjs`) +```
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/js-example-nodejs/README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
packages/js-example-nodejs/README.md (1)
Learnt from: tbouffard
PR: maxGraph/maxGraph#826
File: packages/js-example-nodejs/src/index.cjs:64-69
Timestamp: 2025-05-13T12:54:55.196Z
Learning: For example code in the maxGraph repository, maintainers prefer to keep scripts simple without error handling to focus on demonstrating core functionality, especially in demonstration scripts like those in packages/js-example-nodejs.
🪛 markdownlint-cli2 (0.17.2)
packages/js-example-nodejs/README.md
13-13: No empty links
null
(MD042, no-empty-links)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/js-example-nodejs/src/index.mjs (2)
21-21: Disable ESLint rule for console statements in this example.
This demo usesconsole.info()for output. Suppress theno-consoleESLint rule with an inline comment:+/* eslint-disable no-console */🧰 Tools
🪛 ESLint
[error] 21-21: Unexpected console statement.
(no-console)
23-23: Enhance the example with a simple usage demonstration.
After instantiating the graph, consider logging or exporting something to show the graph in action:-const graph = new BaseGraph(); +const graph = new BaseGraph(); // Simple demonstration: log the graph instance +console.log('Graph instance created:', graph);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/js-example-nodejs/README.md(1 hunks)packages/js-example-nodejs/src/index.mjs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/js-example-nodejs/README.md
🧰 Additional context used
🧠 Learnings (1)
packages/js-example-nodejs/src/index.mjs (1)
Learnt from: tbouffard
PR: maxGraph/maxGraph#826
File: packages/js-example-nodejs/src/index.cjs:64-69
Timestamp: 2025-05-13T12:54:55.196Z
Learning: For example code in the maxGraph repository, maintainers prefer to keep scripts simple without error handling to focus on demonstrating core functionality, especially in demonstration scripts like those in packages/js-example-nodejs.
🪛 ESLint
packages/js-example-nodejs/src/index.mjs
[error] 21-21: Unexpected console statement.
(no-console)
🔇 Additional comments (1)
packages/js-example-nodejs/src/index.mjs (1)
1-15: Standard Apache 2.0 license header requires no review.
|
The new commonjs resources are built with a new tsc command and now referenced in the package.json file.
Add new examples using CommonJS with Node.js to demonstrate the new support:
maxGraphin a headless environment with Node.js (and jsdom).This example exports Graph Model as XML, and exports the rendered Graph to SVG.
In addition, add missing git extension in the repository attribute of the package.json file.
Notes
Closes #790
Closes #824
Covers #827
The implementation is inspired by https://medium.com/ekino-france/supporting-dual-package-for-cjs-and-esm-in-typescript-library-b5feabac1357
The size of the examples remains unchanged compared to #822.
See logs of the GH workflow run on commit 57c250e 👇🏿 https://github.com/maxGraph/maxGraph/actions/runs/14990485089/job/42112503223#step:9:249
Logs of the GH workflow run
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Documentation
Chores