feat(facade): add unit hooks #3158
Conversation
| for (const callback of this.beforeUnitAddCallbacks) { | ||
| if (callback(unit) === false) { | ||
| // if not use throw error, the createUnit returns null | ||
| throw new Error('[UniverInstanceService]: cannot add unit with user callback.'); |
There was a problem hiding this comment.
@wzhudev Check if this has been handled properly.
There was a problem hiding this comment.
I don't know. It is your responsibility to check if it is handled properly.
View Deployment
|
| }); | ||
|
|
||
| // TODO: not calling dispose() method | ||
| // unit.dispose(); |
There was a problem hiding this comment.
@wzhudev The unit.dispose() function is not associated; in the future, a better and more unified mechanism is needed. Consider moving dispose to UnitModel or an intermediate layer. Use dispose for the destroy function.
There was a problem hiding this comment.
Already clearified this with hexf00 offline.
hexf00
force-pushed
the
feat/facade-unit-hooks
branch
from
8a0b16c to
0e106b7
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #3158 +/- ##
==========================================
+ Coverage 27.63% 27.65% +0.01%
==========================================
Files 1984 1985 +1
Lines 105556 105605 +49
Branches 22792 22795 +3
==========================================
+ Hits 29172 29200 +28
- Misses 76384 76405 +21 ☔ View full report in Codecov by Sentry. |
hexf00
force-pushed
the
feat/facade-unit-hooks
branch
from
0e106b7 to
7423d7c
Compare
| * It also manages the focused univer instance. | ||
| */ | ||
| export interface IUniverInstanceService { | ||
| /** A set of callbacks that will be called before a new unit is added. */ |
There was a problem hiding this comment.
Should never expose internal properties as mutable. Instead, provide methods to do that.
| // TODO: not calling dispose() method | ||
| // unit.dispose(); | ||
|
|
||
| const _univerInstanceService = get(IUniverInstanceService); |
There was a problem hiding this comment.
Not necessary to add an underscore before a variable. The underbar has a special usage for "not used variables".
| }); | ||
|
|
||
| // TODO: not calling dispose() method | ||
| // unit.dispose(); |
There was a problem hiding this comment.
Already clearified this with hexf00 offline.
2 participants
Hooks naming will be standardized to the format: prefix + verb in present tense + noun.
Pull Request Checklist