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

Remove reliance on CamelCase component names #9

Closed

Conversation

Archmonger
Copy link
Contributor

@Archmonger Archmonger commented Oct 29, 2021

is_component_def now checks if a function is decorated with @idom.component to determine whether something is a component.

This implementation also works if directly using @component as a decorator.

Also, all hook names are now explicitly checked for rather than assuming everything use_* is a hook.

@Archmonger
Copy link
Contributor Author

Archmonger commented Oct 29, 2021

Accidentally PR'd the archived repo. Moving to idom-team/flake8-idom#3

@Archmonger Archmonger closed this Oct 29, 2021
@rmorshea
Copy link
Contributor

I unarchived this a while back. Haven't had time to switch to flake8-idom.

@rmorshea rmorshea reopened this Oct 30, 2021
@rmorshea
Copy link
Contributor

rmorshea commented Nov 3, 2021

A note on this this - one reason using the CamelCase for function component definitions makes sense is that the behavior is much more like that of a class. If for example, we named these with snake case one might expect the following to be true:

@component
def my_component():
    print("It ran!")
    return idom.html.div()

assert my_component() == idom.html.div()

However, in reality:

assert isinstance(my_component(), Component)

We also do not see the print statement.

To get the initially expected result we must do manually, what the layout automatically would otherwise:

instance = my_component()
assert instance.render() == idom.html.div()

STDOUT:

It ran!

The take away here being that the naming of function components with CamelCase is more than just stylistic. It is an attempt to indicate a behavioral difference between functions that are/aren't components - functions named with CamelCase behave much more like a class (i.e. it's an object constructor).

@Archmonger
Copy link
Contributor Author

To end users, that's irrelevant.

Functions def = snake_case
Class def = CamelCase

Regardless of what they mutate into due to the decorator, I believe PEP8 should be followed.

Plus we can't expect most users to use the flake8-idom package. Many will just use a pylint-only configuration.

flake8_idom_hooks/utils.py Outdated Show resolved Hide resolved
flake8_idom_hooks/utils.py Outdated Show resolved Hide resolved
@rmorshea
Copy link
Contributor

rmorshea commented Nov 3, 2021

You'll need to update the test cases in this file to use an @component decorator for the tests to pass: https://github.com/idom-team/flake8-idom-hooks/blob/main/tests/hook_usage_test_cases.py

@Archmonger Archmonger marked this pull request as ready for review August 19, 2022 09:54
Copy link
Contributor

@rmorshea rmorshea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM besides this. Will merge and release next week.

flake8_idom_hooks/utils.py Outdated Show resolved Hide resolved
requirements/test.txt Outdated Show resolved Hide resolved
@rmorshea
Copy link
Contributor

closed in favor of #14

@rmorshea rmorshea closed this Sep 15, 2022
@Archmonger Archmonger deleted the remove-reliance-on-CamelCase branch September 15, 2022 05:17
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 this pull request may close these issues.

2 participants