Props blacklist

dash/CHANGELOG.md

@@ -1,5 +1,7 @@
 ## Unreleased
 ### Changed
+- [#753](https://github.com/plotly/dash/pull/753) `Component` no longer inherits `MutableMapping`, so `values`, `keys`, and more are no longer methods. This fixed an issue reported in [dcc](https://github.com/plotly/dash-core-components/issues/440) where components with certain prop names defined but not provided would cause a failure to render. During component generation we now disallow all props with leading underscores or matching a few remaining reserved words: `UNDEFINED`, `REQUIRED`, `to_plotly_json`, `available_properties`, and `available_wildcard_properties`.
+
 - [#739](https://github.com/plotly/dash/pull/739) Allow the Flask app to be provided to Dash after object initialization. This allows users to define Dash layouts etc when using the app factory pattern, or any other pattern that inhibits access to the app object. This broadly complies with the flask extension API, allowing Dash to be considered as a Flask extension where it needs to be.
 
 - [#722](https://github.com/plotly/dash/pull/722) Assets are served locally by default. Both JS scripts and CSS files are affected. This improves robustness and flexibility in numerous situations, but in certain cases initial loading could be slowed. To restore the previous CDN serving, set `app.scripts.config.serve_locally = False` (and similarly with `app.css`, but this is generally less important).

dash/dash.py

@@ -914,7 +914,8 @@ def _value_is_valid(val):
         def _validate_value(val, index=None):
             # val is a Component
             if isinstance(val, Component):
-                for p, j in val.traverse_with_paths():
+                # pylint: disable=protected-access
+                for p, j in val._traverse_with_paths():
                     # check each component value in the tree
                     if not _value_is_valid(j):
                         _raise_invalid(
@@ -1194,7 +1195,8 @@ def _validate_layout(self):
         layout_id = getattr(self.layout, 'id', None)
 
         component_ids = {layout_id} if layout_id else set()
-        for component in to_validate.traverse():
+        # pylint: disable=protected-access
+        for component in to_validate._traverse():
             component_id = getattr(component, 'id', None)
             if component_id and component_id in component_ids:
                 raise exceptions.DuplicateIdError(

dash/development/base_component.py

@@ -59,7 +59,7 @@ def _check_if_has_indexable_children(item):
 
 
 @six.add_metaclass(ComponentMeta)
-class Component(patch_collections_abc('MutableMapping')):
+class Component(object):
     class _UNDEFINED(object):
         def __repr__(self):
             return 'undefined'
@@ -184,7 +184,7 @@ def _get_set_or_delete(self, id, operation, new_item=None):
         # If we were in a list, then this exception will get caught
         raise KeyError(id)
 
-    # Supply ABC methods for a MutableMapping:
+    # Magic methods for a mapping interface:
     # - __getitem__
     # - __setitem__
     # - __delitem__
@@ -208,12 +208,12 @@ def __delitem__(self, id):  # pylint: disable=redefined-builtin
         """Delete items by ID in the tree of children."""
         return self._get_set_or_delete(id, 'delete')
 
-    def traverse(self):
+    def _traverse(self):
         """Yield each item in the tree."""
-        for t in self.traverse_with_paths():
+        for t in self._traverse_with_paths():
             yield t[1]
 
-    def traverse_with_paths(self):
+    def _traverse_with_paths(self):
         """Yield each item with its path in the tree."""
         children = getattr(self, 'children', None)
         children_type = type(children).__name__
@@ -224,7 +224,8 @@ def traverse_with_paths(self):
         # children is just a component
         if isinstance(children, Component):
             yield "[*] " + children_string, children
-            for p, t in children.traverse_with_paths():
+            # pylint: disable=protected-access
+            for p, t in children._traverse_with_paths():
                 yield "\n".join(["[*] " + children_string, p]), t
 
         # children is a list of components
@@ -238,12 +239,13 @@ def traverse_with_paths(self):
                 yield list_path, i
 
                 if isinstance(i, Component):
-                    for p, t in i.traverse_with_paths():
+                    # pylint: disable=protected-access
+                    for p, t in i._traverse_with_paths():
                         yield "\n".join([list_path, p]), t
 
     def __iter__(self):
         """Yield IDs in the tree of children."""
-        for t in self.traverse():
+        for t in self._traverse():
             if (isinstance(t, Component) and
                     getattr(t, 'id', None) is not None):
 

dash/development/component_generator.py

@@ -19,6 +19,15 @@
 from ._py_components_generation import generate_classes_files
 
 
+forbidden_props = [
+    'UNDEFINED',
+    'REQUIRED',
+    'to_plotly_json',
+    'available_properties',
+    'available_wildcard_properties'
+]
+
+
 class _CombinedFormatter(
         argparse.ArgumentDefaultsHelpFormatter,
         argparse.RawDescriptionHelpFormatter
@@ -47,9 +56,15 @@ def generate_components(
 
     extract_path = pkg_resources.resource_filename("dash", "extract-meta.js")
 
+    forbid_patterns = '|'.join(
+        '^{}$'.format(p) for p in forbidden_props + ['_.*']
+    )
+
     os.environ["NODE_PATH"] = "node_modules"
     cmd = shlex.split(
-        "node {} {} {}".format(extract_path, ignore, components_source),
+        "node {} {} {} {}".format(
+            extract_path, ignore, forbid_patterns, components_source
+        ),
         posix=not is_windows,
     )
 
@@ -138,8 +153,8 @@ def cli():
         "-p",
         "--package-info-filename",
         default="package.json",
-        help="The filename of the copied `package.json` \
-to `project_shortname`",
+        help="The filename of the copied `package.json` "
+        "to `project_shortname`",
     )
     parser.add_argument(
         "-i",

dash/extract-meta.js

@@ -4,8 +4,11 @@ const fs = require('fs');
 const path = require('path');
 const reactDocs = require('react-docgen');
 
-const componentPaths = process.argv.slice(3);
+const componentPaths = process.argv.slice(4);
 const ignorePattern = new RegExp(process.argv[2]);
+const forbiddenProps = process.argv[3].split('|').map(part => new RegExp(part));
+
+let failed = false;
 
 const excludedDocProps = [
     'setProps', 'id', 'className', 'style'
@@ -20,13 +23,18 @@ const metadata = Object.create(null);
 componentPaths.forEach(componentPath =>
     collectMetadataRecursively(componentPath)
 );
-writeOut(metadata);
+if (failed) {
+    console.error('\nextract-meta failed.\n');
+}
+else {
+    writeOut(metadata);
+}
 
 function help() {
     console.error('usage: ');
     console.error(
-        'extract-meta path/to/component(s) ' +
-            ' [path/to/more/component(s), ...] > metadata.json'
+        'extract-meta ^fileIgnorePattern ^forbidden$|^props$|^patterns$' +
+        ' path/to/component(s) [path/to/more/component(s) ...] > metadata.json'
     );
 }
 
@@ -55,6 +63,20 @@ function docstringWarning(doc) {
     );
 }
 
+function propError(doc) {
+    for(const propName in doc.props) {
+        forbiddenProps.forEach(forbiddenPattern => {
+            if (forbiddenPattern.test(propName)) {
+                process.stderr.write(
+                    `\nERROR: "${propName}" matches forbidden prop name ` +
+                    `pattern: ${forbiddenPattern.toString()}\n`
+                );
+                failed = true;
+            }
+        });
+    }
+}
+
 
 function parseFile(filepath) {
     const urlpath = filepath.split(path.sep).join('/');
@@ -68,6 +90,7 @@ function parseFile(filepath) {
         src = fs.readFileSync(filepath);
         const doc = metadata[urlpath] = reactDocs.parse(src);
         docstringWarning(doc);
+        propError(doc);
     } catch (error) {
         writeError(error, filepath);
     }