-
Notifications
You must be signed in to change notification settings - Fork 29
Description
When a mapping is activated, mapping.ts will dispatch finishMappingInitializationAction once the texture is updated. However, that code is not executed in the tests. For that reason, handleSetMapping in mapping_saga.ts uses this code:
if (process.env.IS_TESTING) {
yield put(finishMappingInitializationAction(layerName));
}However, this IS_TESTING related code is not ideal and should be refactored in my opinion.
When SET_MAPPING is dispatched with the same mapping that is currently active (so, a no-op), mapping.ts will not dispatch a finishMappingInitializationAction, because the listenToStoreProperty listener will not trigger (as the state did not change). however, the reducer will set the mapping state to ACTIVATING.
To circumvent this, updateMappingWithMerge in proofread_saga.ts currently avoids dispatching the no-op action. however, this could be solved more elegantly. quoting from the code:
/* TODO #9064: in case setMappingAction is called with the same mapping
* as already active, the reducer will set the state to ACTIVATING
* but the listenToStoreProperty handler in mappings.ts will never be
* triggered, because the callback is only called if the identity of the
* watched property changes.
* three possible solutions:
* a) avoid dispatching setMappingAction when the mapping did not change
* (this is the current solution here).
* b) Don't set the state to activating in the reducer if the mapping identity,
* did not change.
* (I feel like this makes the logic that controls the lifecycle of the mapping status
* more complicated?)
* c) Refactor the mappings.ts code so that it reacts to all setMapping actions.
* (for example, this could happen in a saga. this would also solve the problem
* that the mapping_saga currently dispatches finishMappingInitializationAction
* when IS_TESTING is true).
* <-- my favorite
*/