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: unnecessary feature module load #631

Merged
merged 8 commits into from
Mar 26, 2021

Conversation

dhhyi
Copy link
Collaborator

@dhhyi dhhyi commented Mar 21, 2021

PR Type

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[x] Refactoring (no functional changes, no API changes)
[x] Build-related changes
[ ] CI-related changes
[ ] Documentation content changes
[x] Application / infrastructure changes
[x] Other: Performance Improvement

What Is the Current Behavior?

In version 0.28 of the PWA there are feature modules loaded even if the features are deactivated:

Go to https://intershoppwa.azurewebsites.net/home and open the 'Network' tab of the developer tools and filter for 'JS' files.

image

In this case the modules for tacton and punchout are loaded even if the features are not active. This also happens for the sentry feature if run locally.

What Is the New Behavior?

Only active feature modules are pulled. For local development with default settings:

image

Does this PR Introduce a Breaking Change?

[ ] Yes
[x] No

Other Information

  • the pulling for tacton and punchout was caused by ProductContextDisplayPropertiesServices, that were bundled in the feature module, but provided in the feature-export modules.
    • I adapted tslint rules so this doesn't happen again
    • I also had to move the captcha sitekey-provider.service to match this pattern
  • After some restructuring, also the sentry feature could be persuaded to load lazily again
    • I streamlined the way extension exports are used, with this I dissolved the core/extras.module.ts as it was mainly undocumented and not used consistently
  • I also reworked the chunk splitting declaration to be more understandable
    • tracking is also a feature module now, as it is extended in projects

After all reviewing and additional work, please squash-merge.

@dhhyi dhhyi added the bug Something isn't working label Mar 21, 2021
@dhhyi dhhyi marked this pull request as ready for review March 21, 2021 23:05
@dhhyi dhhyi requested a review from shauke March 21, 2021 23:05
@shauke shauke self-assigned this Mar 22, 2021
@shauke shauke added this to the 0.29 milestone Mar 22, 2021
@shauke shauke merged commit 3da5f49 into develop Mar 26, 2021
@shauke shauke deleted the fix/unnecessary-feature-module-load branch March 26, 2021 08:43
shauke pushed a commit that referenced this pull request Mar 26, 2021
* chore: optimize rule for deeper lying artifacts in modules
* refactor: move site-key provider service to captcha exports
* fix: move product-context-display-properties services to export folders
* refactor: load multiple lazy modules from the same feature by tracking with type
* refactor: dissolve extras module and align common extension exports format
* fix: restructure sentry feature for lazy loading
* refactor: optimize chunk splitting instructions
* chore: disable unused sentry and tracking for GitHub demo server
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants