Merge pull request #1212 from plotly/missing-inputs

Missing inputs

Author: alexcjohnson

CHANGELOG.md

@@ -8,6 +8,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 - [#1078](https://github.com/plotly/dash/pull/1078) Permit usage of arbitrary file extensions for assets within component libraries
 
 ### Fixed
+- [#1212](https://github.com/plotly/dash/pull/1212) Fixes [#1200](https://github.com/plotly/dash/issues/1200) - prior to Dash 1.11, if none of the inputs to a callback were on the page, it was not an error. This was, and is now again, treated as though the callback raised PreventUpdate. The one exception to this is with pattern-matching callbacks, when every Input uses a multi-value wildcard (ALL or ALLSMALLER), and every Output is on the page. In that case the callback fires as usual.
 - [#1201](https://github.com/plotly/dash/pull/1201) Fixes [#1193](https://github.com/plotly/dash/issues/1193) - prior to Dash 1.11, you could use `flask.has_request_context() == False` inside an `app.layout` function to provide a special layout containing all IDs for validation purposes in a multi-page app. Dash 1.11 broke this when we moved most of this validation into the renderer. This change makes it work again.
 
 ## [1.11.0] - 2020-04-10

CONTRIBUTING.md

@@ -19,6 +19,8 @@ $ cd dash-renderer
 $ npm run build  # or `renderer build`
 # install dash-renderer for development
 $ pip install -e .
+# build and install components used in tests
+$ npm run setup-tests
 # you should see both dash and dash-renderer are pointed to local source repos
 $ pip list | grep dash
 ```

dash-renderer/src/actions/index.js

@@ -136,51 +136,44 @@ function moveHistory(changeType) {
 }
 
 function unwrapIfNotMulti(paths, idProps, spec, anyVals, depType) {
+    let msg = '';
+
     if (isMultiValued(spec)) {
-        return idProps;
+        return [idProps, msg];
     }
+
     if (idProps.length !== 1) {
         if (!idProps.length) {
-            if (typeof spec.id === 'string') {
-                throw new ReferenceError(
-                    'A nonexistent object was used in an `' +
-                        depType +
-                        '` of a Dash callback. The id of this object is `' +
-                        spec.id +
-                        '` and the property is `' +
-                        spec.property +
-                        '`. The string ids in the current layout are: [' +
-                        keys(paths.strs).join(', ') +
-                        ']'
-                );
-            }
-            // TODO: unwrapped list of wildcard ids?
-            // eslint-disable-next-line no-console
-            console.log(paths.objs);
-            throw new ReferenceError(
+            const isStr = typeof spec.id === 'string';
+            msg =
                 'A nonexistent object was used in an `' +
-                    depType +
-                    '` of a Dash callback. The id of this object is ' +
-                    JSON.stringify(spec.id) +
-                    (anyVals ? ' with MATCH values ' + anyVals : '') +
-                    ' and the property is `' +
-                    spec.property +
-                    '`. The wildcard ids currently available are logged above.'
-            );
-        }
-        throw new ReferenceError(
-            'Multiple objects were found for an `' +
+                depType +
+                '` of a Dash callback. The id of this object is ' +
+                (isStr
+                    ? '`' + spec.id + '`'
+                    : JSON.stringify(spec.id) +
+                      (anyVals ? ' with MATCH values ' + anyVals : '')) +
+                ' and the property is `' +
+                spec.property +
+                (isStr
+                    ? '`. The string ids in the current layout are: [' +
+                      keys(paths.strs).join(', ') +
+                      ']'
+                    : '`. The wildcard ids currently available are logged above.');
+        } else {
+            msg =
+                'Multiple objects were found for an `' +
                 depType +
                 '` of a callback that only takes one value. The id spec is ' +
                 JSON.stringify(spec.id) +
                 (anyVals ? ' with MATCH values ' + anyVals : '') +
                 ' and the property is `' +
                 spec.property +
                 '`. The objects we found are: ' +
-                JSON.stringify(map(pick(['id', 'property']), idProps))
-        );
+                JSON.stringify(map(pick(['id', 'property']), idProps));
+        }
     }
-    return idProps[0];
+    return [idProps[0], msg];
 }
 
 function startCallbacks(callbacks) {
@@ -247,20 +240,51 @@ async function fireReadyCallbacks(dispatch, getState, callbacks) {
 
         let payload;
         try {
-            const outputs = allOutputs.map((out, i) =>
-                unwrapIfNotMulti(
+            const inVals = fillVals(paths, layout, cb, inputs, 'Input', true);
+
+            const preventCallback = () => {
+                removeCallbackFromPending();
+                // no server call here; for performance purposes pretend this is
+                // a clientside callback and defer fireNext for the end
+                // of the currently-ready callbacks.
+                hasClientSide = true;
+                return null;
+            };
+
+            if (inVals === null) {
+                return preventCallback();
+            }
+
+            const outputs = [];
+            const outputErrors = [];
+            allOutputs.forEach((out, i) => {
+                const [outi, erri] = unwrapIfNotMulti(
                     paths,
                     map(pick(['id', 'property']), out),
                     cb.callback.outputs[i],
                     cb.anyVals,
                     'Output'
-                )
-            );
+                );
+                outputs.push(outi);
+                if (erri) {
+                    outputErrors.push(erri);
+                }
+            });
+            if (outputErrors.length) {
+                if (flatten(inVals).length) {
+                    refErr(outputErrors, paths);
+                }
+                // This case is all-empty multivalued wildcard inputs,
+                // which we would normally fire the callback for, except
+                // some outputs are missing. So instead we treat it like
+                // regular missing inputs and just silently prevent it.
+                return preventCallback();
+            }
 
             payload = {
                 output,
                 outputs: isMultiOutputProp(output) ? outputs : outputs[0],
-                inputs: fillVals(paths, layout, cb, inputs, 'Input'),
+                inputs: inVals,
                 changedPropIds: keys(cb.changedPropIds),
             };
             if (cb.callback.state.length) {
@@ -360,14 +384,18 @@ async function fireReadyCallbacks(dispatch, getState, callbacks) {
             updatePending(pendingCallbacks, without(updated, allPropIds));
         }
 
-        function handleError(err) {
+        function removeCallbackFromPending() {
             const {pendingCallbacks} = getState();
             if (requestIsActive(pendingCallbacks, resolvedId, requestId)) {
                 // Skip all prop updates from this callback, and remove
                 // it from the pending list so callbacks it was blocking
                 // that have other changed inputs will still fire.
                 updatePending(pendingCallbacks, allPropIds);
             }
+        }
+
+        function handleError(err) {
+            removeCallbackFromPending();
             const outputs = payload
                 ? map(combineIdAndProp, flatten([payload.outputs])).join(', ')
                 : output;
@@ -398,10 +426,13 @@ async function fireReadyCallbacks(dispatch, getState, callbacks) {
     return hasClientSide ? fireNext().then(done) : done;
 }
 
-function fillVals(paths, layout, cb, specs, depType) {
+function fillVals(paths, layout, cb, specs, depType, allowAllMissing) {
     const getter = depType === 'Input' ? cb.getInputs : cb.getState;
-    return getter(paths).map((inputList, i) =>
-        unwrapIfNotMulti(
+    const errors = [];
+    let emptyMultiValues = 0;
+
+    const inputVals = getter(paths).map((inputList, i) => {
+        const [inputs, inputError] = unwrapIfNotMulti(
             paths,
             inputList.map(({id, property, path: path_}) => ({
                 id,
@@ -411,8 +442,45 @@ function fillVals(paths, layout, cb, specs, depType) {
             specs[i],
             cb.anyVals,
             depType
-        )
-    );
+        );
+        if (isMultiValued(specs[i]) && !inputs.length) {
+            emptyMultiValues++;
+        }
+        if (inputError) {
+            errors.push(inputError);
+        }
+        return inputs;
+    });
+
+    if (errors.length) {
+        if (
+            allowAllMissing &&
+            errors.length + emptyMultiValues === inputVals.length
+        ) {
+            // We have at least one non-multivalued input, but all simple and
+            // multi-valued inputs are missing.
+            // (if all inputs are multivalued and all missing we still return
+            // them as normal, and fire the callback.)
+            return null;
+        }
+        // If we get here we have some missing and some present inputs.
+        // Or all missing in a context that doesn't allow this.
+        // That's a real problem, so throw the first message as an error.
+        refErr(errors, paths);
+    }
+
+    return inputVals;
+}
+
+function refErr(errors, paths) {
+    const err = errors[0];
+    if (err.indexOf('logged above') !== -1) {
+        // Wildcard reference errors mention a list of wildcard specs logged
+        // TODO: unwrapped list of wildcard ids?
+        // eslint-disable-next-line no-console
+        console.error(paths.objs);
+    }
+    throw new ReferenceError(err);
 }
 
 function handleServerside(config, payload, hooks) {

package.json

@@ -18,7 +18,8 @@
     "format": "run-s private::format.*",
     "initialize": "run-s private::initialize.*",
     "lint": "run-s private::lint.*",
-    "test.integration": "run-s private::test.setup-* private::test.integration-*",
+    "setup-tests": "run-s private::test.setup-*",
+    "test.integration": "run-s setup-tests private::test.integration-*",
     "test.unit": "run-s private::test.unit-**"
   },
   "devDependencies": {