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

Support for non-serializable component parameters #120

Merged
merged 57 commits into from Feb 2, 2023

Conversation

Archmonger
Copy link
Contributor

@Archmonger Archmonger commented Jan 10, 2023

Description

Functional implementation of database-backed component params

Changelog

  • Database backed component parameters
    • The component template tag now supports both positional and keyword arguments.
    • The component template tag now supports non-serializable arguments.
    • It is now mandatory to run manage.py migrate after installing IDOM
    • IDOM_WS_MAX_RECONNECT_TIMEOUT has been renamed to IDOM_RECONNECT_MAX and is now used as the default timeout for component parameters
  • Bumped the minimum IDOM version to 0.43.0
    • idom.backend.hooks support.
    • django_idom.types.IdomWebsocket has been renamed to Connection and modified to fit the new schema
    • view_to_component utility will now automatically use del_html_head_body_transform
    • Set IDOM_DEBUG_MODE to settings.py:DEBUG
  • The react client is now set to production rather than development
  • Make a harmless utils public but undocumented
  • Documentation for django_query_postprocessor

Checklist:

Please update this checklist as you complete each item:

  • Tests have been included for all bug fixes or added functionality.
  • The changelog has been updated with any significant changes, if necessary.
  • GitHub Issues which may be closed by this PR have been linked.

@Archmonger
Copy link
Contributor Author

@rmorshea This is fully functional but currently has no eviction/expiration strategy implemented. Let me know what you think.

@Archmonger Archmonger linked an issue Jan 10, 2023 that may be closed by this pull request
@rmorshea
Copy link
Contributor

Will take a look tomorrow

@rmorshea
Copy link
Contributor

Overall LGTM. Perhaps we could add a test which passes a parameter to a component that isn't JSON serializable?

@Archmonger
Copy link
Contributor Author

Archmonger commented Jan 11, 2023

The reconnect variable has been changed to better describe the new behavior. Also, the rename helps in preparation for reactive-python/reactpy#525

In the future, I hope to introduce a IDOM_RECONNECT_INTERVAL setting as well.

@Archmonger Archmonger changed the title Database-backed component params Support for non-serializable component parameters Jan 11, 2023
@Archmonger
Copy link
Contributor Author

Changelog in the PR's description has been updated.

@Archmonger
Copy link
Contributor Author

I've added tests for the component parameter cleanup logic

@Archmonger
Copy link
Contributor Author

@rmorshea This has security fixes, so I'd like to get this out soon if possible.

@Archmonger Archmonger changed the title v3.0.0: Support for non-serializable component parameters Support for non-serializable component parameters Jan 28, 2023
@Archmonger
Copy link
Contributor Author

I removed the version bump from this PR.

I will create a follow-up PR that bumps IDOM to 1.0.0 and Django-IDOM to 3.0.0.

@rmorshea
Copy link
Contributor

rmorshea commented Feb 1, 2023

Will review tomorrow.

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.

Generally approve, just few minor nits/comments

docs/src/features/settings.md Show resolved Hide resolved
src/django_idom/apps.py Outdated Show resolved Hide resolved
src/django_idom/templatetags/idom.py Show resolved Hide resolved
@Archmonger
Copy link
Contributor Author

Let me know if we're okay with the approach on the 3 unresolved comments

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.

I think everything looks good to me.

@Archmonger Archmonger merged commit da083ac into reactive-python:main Feb 2, 2023
@Archmonger Archmonger deleted the database-backed-kwargs branch February 2, 2023 06:33
@Archmonger Archmonger restored the database-backed-kwargs branch February 9, 2023 03:37
@Archmonger Archmonger deleted the database-backed-kwargs branch February 21, 2023 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants