Skip to content
This repository was archived by the owner on Jun 1, 2025. It is now read-only.

fix: registered external resouces should keep singleton ref #1320

Merged
merged 3 commits into from
Dec 8, 2023

Conversation

ghiscoding
Copy link
Owner

  • external resources can be provided through the grid options, but these options are sometime deep copied and that might have the side effect of losing the singleton ref of the instantiate services/resources, instead we can simply keep these singleton refs before any grid options merge can happen and that is in the constructor before the grid initializes and before the grid options are merged and sometime deep copied with global options

TODOs

  • requires new version of Slickgrid-Universal

- external resources can be provided through the grid options, but these options are sometime deep copied and that might have the side effect of losing the singleton ref of the instantiate services/resources, instead we can simply keep these singleton refs before any grid options merge can happen and that is in the constructor before the grid initializes and before the grid options are merged and sometime deep copied with global options
@ghiscoding ghiscoding marked this pull request as draft December 2, 2023 01:37
@ghiscoding ghiscoding marked this pull request as ready for review December 8, 2023 03:34
Copy link

codecov bot commented Dec 8, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (567f47b) 100.00% compared to head (e82c9f4) 100.00%.
Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #1320   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            7         7           
  Lines          896       908   +12     
  Branches       302       304    +2     
=========================================
+ Hits           896       908   +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ghiscoding ghiscoding merged commit f339163 into master Dec 8, 2023
@ghiscoding ghiscoding deleted the bugfix/registered-resources-deep-copied branch December 8, 2023 04:01
ghiscoding added a commit that referenced this pull request Dec 15, 2023
- fixes issue #1329
- external resources can also be loaded globally in the forRoot and that regressed in previous PR #1320
ghiscoding added a commit that referenced this pull request Dec 16, 2023
- partially rolls back PR #1320  defining a global external resource for ExcelExport no longer worked.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant