Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] dash-generate-components does no longer recognize imported object in propTypes #2096

Closed
rubenthoms opened this issue Jun 14, 2022 · 4 comments · Fixed by #2097
Closed

Comments

@rubenthoms
Copy link

Describe your context
structure.tsx

import PropTypes from "prop-types";

export type Structure = { test: string; };
export const StructurePropTypes = { test: PropTypes.string.isRequired };

component.tsx

import React from "react";
import PropTypes from "prop-types";

import { Structure, StructurePropTypes } from "./structure";

type ComponentProps = { 
    structure: Structure;
};

const Component: React.FC<ComponentProps> = (props) => {
    ...
};

Component.propTypes = {
    structure: PropTypes.shape(StructurePropTypes).isRequired,
};
  • replace the result of pip list | grep dash below
dash                 2.0.0 vs 2.5.1 (others are not relevant)

Describe the bug

Given the example above and running dash-generate-components with dash==2.5.1 results in:

Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.8.12/x64/bin/dash-generate-components", line 8, in <module>
    sys.exit(cli())
  File "/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/dash/development/component_generator.py", line 242, in cli
    generate_components(
  File "/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/dash/development/component_generator.py", line 136, in generate_components
    components = generate_classes_files(project_shortname, metadata, *generator_methods)
  File "/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/dash/development/_py_components_generation.py", line 195, in generate_classes_files
    generator(
  File "/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/dash/development/_py_components_generation.py", line 166, in generate_class_file
    class_string = generate_class_string(
  File "/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/dash/development/_py_components_generation.py", line 125, in generate_class_string
    nodes = collect_nodes({k: v for k, v in props.items() if k != "children"})
  File "/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/dash/development/_collect_nodes.py", line 49, in collect_nodes
    nodes = collect_nodes(t_value["value"], key, nodes)
  File "/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/dash/development/_collect_nodes.py", line 34, in collect_nodes
    for prop_name, value in metadata.items():
AttributeError: 'str' object has no attribute 'items'

It seems dash-generate-components treats StructurePropTypes as a string and no longer as an imported object.

However, when executing the same command with dash==2.0.0 there is no exception raised.

Expected behavior

I expect dash-generate-components to be able to correctly treat imported objects in propTypes. I'd like to store more complex objects describing the type of individual properties that are used by multiple components in additional files. I do not want to have the same object definition multiple places in my code.

It might be that the newest version of dash is more strict about the analysis of propTypes and that the older version did just ignore the error (did not encounter it because of a less deeper analysis). However, it would be nice to have this supported in dash.

@T4rk1n
Copy link
Contributor

T4rk1n commented Jun 14, 2022

In tsx files you have to use typescript props, I think there is a conflict in the props resolution in this case since you have the props in typescript in the example. So you can remove the Component.propType = {...} and just use the type ComponentProps = {structure: Structure;}; and that should work.

@rubenthoms
Copy link
Author

rubenthoms commented Jun 14, 2022

@T4rk1n thanks for your quick reply. To be clear, I am not using the --ts flag when running dash-generate-components. Is it still possible for the algorithm to encounter a conflict with TypeScript in this case? However, I'm sure your suggestion works (haven't tested it yet since my package is not pure TypeScript yet). Though, I am not completely convinced yet. This does rather sound like a workaround and not a solution to the problem, does it? Please correct me if I'm wrong, but TypeScript performs type-checking solely at compile time while PropTypes acts as a runtime type-checker. Being forced to remove PropTypes in order to make the dash-generate-components algorithm work (when not even intentionally wanting to use TypeScript props - hence, not using the --ts flag) removes useful functionality from the component. Any call to the functional component at runtime (e.g. updating the component with data from an API call where we, of course, can define the expected type in TypeScript but never be certain that we will receive it like that) will be performed without any type-checking. I know that this might be a rather seldom case, but it is a case that can occur and that must be considered. It is especially important for us, since we are using our package both for creating dash components and providing it as a pure npm package.

@T4rk1n
Copy link
Contributor

T4rk1n commented Jun 14, 2022

when not even intentionally wanting to use TypeScript props - hence, not using the --ts flag

The --ts flag was removed before release, the compiler now parse tsx files when it encounters them so you can mix js/tsx components in the same directory. The compiler should have taken the right props from: React.FC<ComponentProps> but it looks like that case doesn't work or there is a conflict with the propTypes that needs to be fixed.

@T4rk1n
Copy link
Contributor

T4rk1n commented Jun 14, 2022

React.FC<Props> doesn't work as reported in #2095 , the new collect_nodes for component as props seems to be eating up the proper error down the line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants