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

Secondary entry points to allow better dead-code elimination #4628

Merged
merged 75 commits into from
Feb 13, 2024

Conversation

FilipLeitner
Copy link
Collaborator

@FilipLeitner FilipLeitner commented Jan 11, 2024

Description

As described in #4616

Related issues or pull requests

closes #4616

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Dependency updates
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe)
  • I am unsure (we'll look into it together)

Do you introduce a breaking change?

  • Yes
  • No
  • I am unsure (no worries, we'll find out)

Checklist

  • I understand and agree that the changes in this PR will be licensed under the [MIT License]
  • I have followed the guidelines for contributing
  • The proposed change fits to the content of the code of conduct
  • I have added or updated tests and documentation, and the test suite passes (run npm test locally)
  • I'm lost; why do I have to check so many boxes? Please help!

@FilipLeitner FilipLeitner requested a review from jmacura as a code owner January 11, 2024 12:45
@FilipLeitner FilipLeitner marked this pull request as draft January 11, 2024 12:46
@FilipLeitner
Copy link
Collaborator Author

FilipLeitner commented Jan 11, 2024

  • hslayers-app should be loading much quicker - its not possible to treeshake the code as we dont know whats gonna be required but with each component having separate chunk the browser will get only whats necessary based on config.panelsEnabled and config.componentsEnabled
    image
    image

  • in 'full app' or app that use hslayers-ng library from npm have different options (described briefly in decouple-test-app project)
    a) treeshake unused code by importing only whats necessary - code will end up in main.js

    image

    b) Use same technique as used in hslayers-app
    c) possibly combination of both eg. dynamic imports in container app directly

Most of the work has been completed.

TBD:

  • hslayers component is basically of no use right now as I was mostly evading imports from main entry point hslayers. It needs to be tested wether it causes apps to bundle whole hslayers (becasue of barel public-api) file or it still can be use along with the principles described above.... If it will be usable we might probably merge it with layout component as we wont need both,
  • Otherwise setting of a config and app id should work without necessity of using hslayers component.
  • try make dependencies entry-point speciffic if possible** eg. allow container app to be compiled with deps that it really needs only
  • some sort of API adjustments and cleanup of project structure as this wasnt really thought through
  • cesium update
  • possibly remove hslayers-material

@FilipLeitner FilipLeitner mentioned this pull request Jan 12, 2024
17 tasks
@FilipLeitner
Copy link
Collaborator Author

FilipLeitner commented Jan 12, 2024

Few new changes.:

  • deprecated layout module in favor of hslayers module which basically took on all of the responsibilites of layout module

  • removed exports from barel main entry poublic-api -> imports will be available from secondary entries only (hslayers component is an exception obviously)

    • As a result components will not be loaded automatically jsut by adding hslayers element to app template. Separate construction methods or manual setup is necessary . The reason is that we would not be able to have both of the options available in case the files with dynamic imports are always present in a bundle
      image
  • made queryPopup a standalone entrypoint so we dont always import all of query if popus are wanted

  • NEW CONFIG VALUE: componentsEnabled.queryPopup which controls whether a popup functionality will be avilable

  • tested vendor treeshaking a bit and it seem to be reacting somehow out of the box. Some unexpected libraries ended up in bundle of reasonanly stripped app - geostylers, loadsh but for example polygon-splitter was not there. Will have to investigate further

@FilipLeitner
Copy link
Collaborator Author

hslayers-ng bundle part size comparison for https://www.agrihub.cz/hsl-ng/AgroClima/
(percentages are not really usable as´"before" app vendors were split somehow, to satisfy webpack hModuleFederation eg. significant part is not included in the same bundle as hsl.
image

Use ngIf instead of hidden and simplify value calculation
BREAKING CHANGE: query component (info panel) renamed to query, info component (compositionLoadingProgress) renamed to info and configuration moved from panels to components
Less verbose internal panels creation

Expose single method for creating external panels with and sb buttons
…mponents

Align layerm-manager files with  naming convention of the repo

BREAKING CHANGE: panelsEnabled options renamed
refactor: Remove @defer usage as currently it doesnt allow treeshaking of the component
build: Improve treeshakability of print component
fix: Print imports
Code distribution changes from per component to  chunk per panel
In contrast with chunk create by less verbose approach + max chunk number plugin content of these chunks are not mixed
fix(sensors): Panel visiblity

refactor: No panel enabled check necessary when switching panels

Only active panels are available
Copy link
Collaborator

@jmacura jmacura left a comment

Choose a reason for hiding this comment

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

I am leaving some comments, but propose to only solve the typos in this PR and to look at the rest at some consecutive PRs so this doesn't grow indefinitely.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why this files goes into shared? This is not a service. Is it because it requires services in its constructor?

Copy link
Collaborator Author

@FilipLeitner FilipLeitner Feb 13, 2024

Choose a reason for hiding this comment

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

Exactly, its imported only via wfs.service which is in the same entry point as this file. Circular dependencies are more strict than we were used to before. Once you have import from shared/add-data in components/add-data you can not go the other way around. Even for totaly separate files.

@jmacura
Copy link
Collaborator

jmacura commented Feb 13, 2024

FTR, this PR also removes hlayers-material, an HSLayers-NG version with Material Design.

@jmacura jmacura merged commit 4684e14 into develop Feb 13, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiple entry points Resurrect hslayers-material
2 participants