[FIX] Fix the type definitions #1814
Conversation
In the npm package, the definitions were previously located in dist/src instead of in dist/. It also contained development/demo types in dist/dev. So, project integrating bpmn-visualization were unable to retrieve the types and got errors like TS7016: Could not find a declaration file for module 'bpmn-visualization' The issue was located in the rollup configuration that override the tsconfig file when building the npm package. The 'include' configuration was not correctly overridden, so we used the development values. This is now fixed and there is a new test that will prevent this from happening again. Some definition files previously referenced types or classes that were not defined. Such elements are marked as internal and strip from the definitions. So this was generating errors like TS1192 or TS2305 (mainly involving BpmnModel, Edge or Shape). There were some functions and classes that weren't marked as @internal and so created the broken references. To improve the usage of bpmn-visualization in TypeScript projects, the README now provides more information about the TypeScript support and how to use the lib in TS projects.
tbouffard
added
bug
|
♻️ PR Preview e841707 has been successfully destroyed since this PR has been closed.
🤖 By surge-preview |
|
♻️ PR Preview e841707 has been successfully destroyed since this PR has been closed.
🤖 By surge-preview |
| - 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]` |
There was a problem hiding this comment.
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.
README.md
Outdated
| Alternatively, you can set `skipLibCheck` to `true` in the `tsconfig.json` file but this limits the definition checks . | ||
| For more details, see the [skipLibCheck documentation](https://www.typescriptlang.org/tsconfig#skipLibCheck). |
There was a problem hiding this comment.
❓ I would like to not propose this alternative, but it avoids the mxgraph bump problem described above. What do you think?
| // Ensure we only bundle production sources | ||
| if (!devMode) { | ||
| tsconfigOverride.include = ['src/**/*']; | ||
| options.tsconfig = './tsconfig.bundle.json'; |
There was a problem hiding this comment.
ℹ️ I haven't found a way to avoid adding a new tsconfig file.
According to the plugin documentation, the include in the override options is merged with the one from the tsconfig file. This prevents us to remove the dev folder from the include.
| @@ -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.
ℹ️ not related to the fix, but I didn't want to create a dedicated PR for this 😸
| @@ -139,6 +139,6 @@ export class ConvertedElements { | |||
| } | |||
| } | |||
|
|
|||
| export interface CategoryValueData { | |||
| interface CategoryValueData { | |||
There was a problem hiding this comment.
ℹ️ I removed the export as the interface is only used in the file.
| `('$bundleType', ({ file }: { file: string }) => { | ||
| expect(existsSync(resolve(bundlesDirectoryPath, file))).toBe(true); | ||
| }); |
There was a problem hiding this comment.
ℹ️ function simplification to avoid extra eslint rule disable
tbouffard
requested review from
a team and
csouchet
and removed request for
a team
Co-authored-by: Souchet Céline <[email protected]>
|
Kudos, SonarCloud Quality Gate passed! |
In the npm package, the definitions were previously located in dist/src instead of in dist/. It also contained development/demo types in dist/dev. So, project integrating bpmn-visualization were unable to retrieve the types and got errors like TS7016: Could not find a declaration file for module 'bpmn-visualization'
The issue was located in the rollup configuration that override the tsconfig file when building the npm package. The 'include' configuration was not correctly overridden, so we used the development values.
This is now fixed and there is a new test that will prevent this from happening again.
Some definition files previously referenced types or classes that were not defined. Such elements are marked as internal and strip from the definitions.
So this was generating errors like TS1192 or TS2305 (mainly involving BpmnModel, Edge or Shape).
There were some functions and classes that weren't marked as
@internaland so created the broken references.To improve the usage of bpmn-visualization in TypeScript projects, the README now provides more information about the TypeScript support and how to use the lib in TS projects.
closes #1808
Notes
./node_modules/.bin/tsc --emitDeclarationOnly --declaration