-
Notifications
You must be signed in to change notification settings - Fork 33
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
[FIX] Fix the type definitions #1814
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -107,6 +107,36 @@ You can set the BPMN content using one of the following ways: | |
* Load it from an url, like this [example](https://github.com/process-analytics/bpmn-visualization-examples/blob/master/examples/display-bpmn-diagram/load-remote-bpmn-diagrams/index.html) | ||
* Load from your computer, like the [demo example](https://github.com/process-analytics/bpmn-visualization-examples/tree/master/examples/display-bpmn-diagram/load-local-bpmn-diagrams/index.html) | ||
|
||
#### TypeScript Support | ||
|
||
`bpmn-visualization` provides type definitions, so the integration should work out of the box in TypeScript projects. | ||
|
||
Depending on the build system used by the TypeScript project, it may get the following type errors | ||
- error TS2688: Cannot find type definition file for 'typed-mxgraph' | ||
- error TS7016: Could not find a declaration file for module 'mxgraph' | ||
|
||
In this case, | ||
- Install `@typed-mxgraph/typed-mxgraph` as devDependencies, for instance by running `npm i --save-dev @typed-mxgraph/[email protected]` | ||
- Declare the `typed-mxgraph` types in the `tsconfig.json` as in the following 👇 | ||
|
||
```json | ||
{ | ||
"compilerOptions": { | ||
"typeRoots": [ | ||
"node_modules/@types", | ||
"node_modules/@typed-mxgraph" | ||
] | ||
} | ||
} | ||
``` | ||
|
||
Alternatively, you can set `skipLibCheck` to `true` in the `tsconfig.json` file but this limits the definition checks . | ||
tbouffard marked this conversation as resolved.
Show resolved
Hide resolved
|
||
For more details, see the [skipLibCheck documentation](https://www.typescriptlang.org/tsconfig#skipLibCheck). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❓ I would like to not propose this alternative, but it avoids the mxgraph bump problem described above. What do you think? |
||
|
||
Advanced users that want to extend the `mxGraph` integration must configure `typed-mxgraph`. | ||
|
||
For more details, see the TypeScript projects in the [bpmn-visualization-examples repository](https://github.com/process-analytics/bpmn-visualization-examples/). | ||
|
||
|
||
### 💠 Browser usage | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -124,15 +124,17 @@ function typescriptPlugin() { | |
const tsSourceMap = !demoMode && !buildBundles; // TODO logic duplicated with build selection | ||
const tsconfigOverride = { compilerOptions: { sourceMap: tsSourceMap, declaration: tsDeclarationFiles } }; | ||
|
||
const options = { | ||
typescript: require('typescript'), | ||
tsconfigOverride: tsconfigOverride, | ||
}; | ||
|
||
// Ensure we only bundle production sources | ||
if (!devMode) { | ||
tsconfigOverride.include = ['src/**/*']; | ||
options.tsconfig = './tsconfig.bundle.json'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ℹ️ I haven't found a way to avoid adding a new tsconfig file. |
||
} | ||
|
||
return typescript({ | ||
typescript: require('typescript'), | ||
tsconfigOverride: tsconfigOverride, | ||
}); | ||
return typescript(options); | ||
} | ||
|
||
function withMinification(plugins) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,9 +14,8 @@ | |
* limitations under the License. | ||
*/ | ||
|
||
import type { GlobalTaskKind, ShapeBpmnSubProcessKind } from '../../../model/bpmn/internal'; | ||
import type { GlobalTaskKind, MessageVisibleKind, ShapeBpmnSubProcessKind } from '../../../model/bpmn/internal'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ℹ️ not related to the fix, but I didn't want to create a dedicated PR for this 😸 |
||
import { ShapeBpmnEventBasedGatewayKind, ShapeBpmnEventDefinitionKind } from '../../../model/bpmn/internal'; | ||
import type { MessageVisibleKind } from '../../../model/bpmn/internal/edge/kinds'; | ||
import { mxgraph } from '../initializer'; | ||
import { BpmnStyleIdentifier } from './identifiers'; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -139,6 +139,6 @@ export class ConvertedElements { | |
} | ||
} | ||
|
||
export interface CategoryValueData { | ||
interface CategoryValueData { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ℹ️ I removed the export as the interface is only used in the file. |
||
value?: string; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,13 +32,10 @@ describe('bundles', () => { | |
${'bpmn-visualization.esm.min.js'} | ${'ESM minified'} | ||
${'bpmn-visualization.js'} | ${'IIFE'} | ||
${'bpmn-visualization.min.js'} | ${'IIFE minified'} | ||
`( | ||
'$bundleType', | ||
// eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
({ file, bundleType }) => { | ||
expect(existsSync(resolve(bundlesDirectoryPath, file))).toBe(true); | ||
}, | ||
); | ||
${'bpmn-visualization.d.ts'} | ${'Type definitions'} | ||
`('$bundleType', ({ file }: { file: string }) => { | ||
expect(existsSync(resolve(bundlesDirectoryPath, file))).toBe(true); | ||
}); | ||
Comment on lines
+36
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ℹ️ function simplification to avoid extra eslint rule disable |
||
}); | ||
|
||
it('IIFE bundle - should generate BPMN Diagram SVG', async () => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
{ | ||
"extends": "./tsconfig", | ||
"include": [ | ||
"src/**/*" | ||
] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am also not sure that it is a good idea to specify the version (more maintenance in the README) because the package is very stable and is not going to receive new versions except if a big issue is found.