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

[FIX] demo code does not interfere with lib integration #479

Merged
merged 10 commits into from
Aug 20, 2020

Conversation

tbouffard
Copy link
Member

@tbouffard tbouffard commented Aug 10, 2020

closes #473

The demo code is now hidden in a module and don't have side effects (globals, require html elements available in the page). The demo bootstrap code specific to the demo page is defined in a dedicated JavaScript script.
To be able to access to the demo and lib code in the demo JS code, we now a 1st set of elements (functions, classes, ....) exported by the lib.

To ensure that there is no more lib integration issue, we have added a dedicated page using specific code that are not using the demo code at all (only the lib code). A new specific e2e view test ensures that this page is working.

The terser compress and mangle configuration has been reactivated: thanks to the new
exports defined in the index.ts, terser is able to generate a minified bundle including
right export names.
The minified bundle size then decreases from 189.64 kb (master 3bf93fb) to 135.85 kb (6df5f79).

We also added some refactoring

  • To reduce the impact of the demo code on the 'non regression' page, we the id of the html element used to render the diagram is no more hard coded and can be passed to the demo processing code. So the 'non regression' page is not forced (as the lib integration page) to define ghost div required by the demo code.
  • We do not load the lib directly in the html pages, it is loaded indirectly by the js app code that requires it (via import)

Notes

@tbouffard tbouffard added bug Something isn't working WIP 🚧 Pull request in progress and/or not ready for review. Used in addition to marking it as draft lib integration Something about how an app can integrate the library labels Aug 10, 2020
@tbouffard tbouffard force-pushed the 473_demo_code_does_not_interfer_on_lib_integration branch 3 times, most recently from fa27a8d to c408cea Compare August 11, 2020 06:44
@tbouffard tbouffard changed the title [FIX] demo code does not interfer on lib integration [FIX] demo code does not interfer with lib integration Aug 11, 2020
@tbouffard tbouffard changed the title [FIX] demo code does not interfer with lib integration [FIX] demo code does not interfere with lib integration Aug 11, 2020
@tbouffard tbouffard force-pushed the 473_demo_code_does_not_interfer_on_lib_integration branch 3 times, most recently from d59b336 to e6f8990 Compare August 19, 2020 14:32
wip: introduce global exports

wip - 1st implem: demo code hidden in ES6 module

demo code work (no alias)

wip: add page to demonstrate that demo has no side effect on lib integration

wip: add page to demonstrate that demo has no side effect on lib integration

WIP add e2e test diagram for the lib-integration

WIP add e2e test for the lib-integration

make lib-integration.html page display the graph

ensure IconPainter is exported

fix e2e test: lib integration page

ensure default from internal modules are exported in the bundle

reactivate terser compress and mangle

thanks to the new es6 exports defined in the index.ts, terser is able to
generate a minified bundle including right export names.

The minified bundle size decreases from 180.99 kb to 129.64 kb.

export{or as BpmnVisualization,lr as DropFileUserInterface,I as IconPainter,O as
IconPainterProvider,i as MarkerIdentifier,N as StyleConfigurator,s as
StyleDefault,a as StyleIdentifier,x as buildPaintParameter,y as
computeScaledIconSize,ir as documentReady,dr as handleFileSelect,ur as loadBpmn,
sr as log,ar as logStartup,pr as readAndLoadFile};
Clarify the code: what is important is now first
Allow to configure the id of the html element used as visualization container.
In the 'non regression' page, no need to define used div and use a custom id
for the viewport that will make the graph fit the page.

# Conflicts:
#	src/demo/index.ts
It is loaded indirectly by the js app code that requires it (via import)
@tbouffard tbouffard force-pushed the 473_demo_code_does_not_interfer_on_lib_integration branch from a5c8dac to 6df5f79 Compare August 20, 2020 09:05
@tbouffard tbouffard removed the WIP 🚧 Pull request in progress and/or not ready for review. Used in addition to marking it as draft label Aug 20, 2020
@tbouffard tbouffard requested a review from aibcmars August 20, 2020 11:39
@tbouffard tbouffard marked this pull request as ready for review August 20, 2020 11:39
@tbouffard tbouffard removed the request for review from aibcmars August 20, 2020 11:41
@tbouffard tbouffard marked this pull request as draft August 20, 2020 11:41
@tbouffard tbouffard requested a review from aibcmars August 20, 2020 11:56
@tbouffard tbouffard marked this pull request as ready for review August 20, 2020 11:56
@tbouffard
Copy link
Member Author

Notice that there are TODO about imports/exports.
We have decided with @aibcmars to not cover them now; we will work on this topic when managing the bundle and npm publishing.

Copy link
Contributor

@aibcmars aibcmars left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

except for the part with import-export in src/index.ts, which we will handle in the near future, everything seems OK - go for merging:)

@tbouffard tbouffard merged commit 98a9e23 into master Aug 20, 2020
@tbouffard tbouffard deleted the 473_demo_code_does_not_interfer_on_lib_integration branch August 20, 2020 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lib integration Something about how an app can integrate the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] the demo code prevents an easy integration
2 participants