Dynamic sitemap generation #3693
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* implement more literal vars * fix super issue * pyright has a bug i think * oh we changed that * fix docs * literalize vars recursively * do what masen told me :D * use dynamic keys * forgot .create * adjust _var_value * dang it darglint * add test for serializing literal vars into js exprs * fix silly mistake * add handling for var and none * use create safe * is none bruh * implement function vars and do various modification * fix None issue * clear a lot of creates that did nothing * add tests to function vars * added simple fix smh * use fconcat to make an even more complicated test
…v#3696) * [REF-3184] Raise exception when encountering nested `async with self` blocks Avoid deadlock when the background task already holds the mutation lock for a given state. * [REF-3339] get_state from background task links to StateProxy When calling `get_state` from a background task, the resulting state instance is wrapped in a StateProxy that is bound to the original StateProxy and shares the same async context, lock, and mutability flag. * If StateProxy has a _self_parent_state_proxy, retrieve the correct substate * test_state fixup
masenf
reviewed
Jul 23, 2024
|
|
||
| async def serve_sitemap(state: State, app: App) -> Response: | ||
| sitemap_content = generate_sitemap(app) | ||
| actual_domain = state.router.session.client_token or "http://localhost:3000" |
There was a problem hiding this comment.
client_token is not a url
avoid hardcoded port on localhost
There was a problem hiding this comment.
Not yet; this is a draft to outline my initial approach. I'll conduct testing and make necessary adjustments before finalizing it. I'm also open to any suggestions you might have.
There was a problem hiding this comment.
iirc you can get the domain with get_config().deploy_url
| sitemap_content = generate_sitemap(app) | ||
| actual_domain = state.router.session.client_token or "http://localhost:3000" | ||
| modified_sitemap = sitemap_content.replace( | ||
| "{{ PLACEHOLDER_DOMAIN }}", f"https://{actual_domain}" |
|
Nice idea! Do you have a link to issue |
* implement var operation decorator * use older syntax * use cast and older syntax * use something even simpler * add some tests * use old union tactics * that's not how you do things * implement arithmetic operations while we're at it * add test * even more operations * can't use __bool__ * thanos snap * forgot ruff * use default factory * dang it darglint * i know i should have done that but * convert values into literalvars * make test pass * use older union tactics * add test to string var * pright why do you hate me 🥺
…reflex-dev#3708) When using rx.vars.get_uuid_string_var, wrap the prop Var in `useMemo` so that the value remains consistent across re-renders of the component. Fix reflex-dev#3707
Prevent a validation error from pydantic/v1 that cannot find _var_name, etc. in __dataclass_fields__
) * fix silly bug when style is set directly to breakpoints * add helpful comment Co-authored-by: Masen Furer <[email protected]> --------- Co-authored-by: Masen Furer <[email protected]>
* half of the way there * implement __getitem__ for array * add some tests * add fixes to pyright * fix default factory * implement array operations * format code * fix pyright issue * give up * add object operations * add test for merge * pyright 🥺 * use str isntead of _var_name Co-authored-by: Masen Furer <[email protected]> * wrong var_type * make to much nicer * add subclass checking * enhance types * use builtin list type * improve typing even more * i'm awaiting october * use even better typing * add hash, json, and guess type method * fix pyright issues * add a test and fix lots of errors * fix pyright once again * add type inference to list --------- Co-authored-by: Masen Furer <[email protected]>
* fix initial_value for computed_var * fix initial_value in pyi
* add type hinting to existing types * dang it darglint * i cannot
Co-authored-by: coolstorm <[email protected]>
When a parent state proxy is set, also allow child StateProxy._self_mutable to override the parent's `_is_mutable()`.
…plate during init (reflex-dev#3750)
There was a problem hiding this comment.
@TG199 thanks for your effort, and sorry for my late review.
| links = [] | ||
|
|
||
| for route, page in app.pages.items(): | ||
| if "[" in route or "]" in route: |
There was a problem hiding this comment.
Dynamic routes should contain both, [ and ]
Add a comment that this ignores dynamic routes for now
| @@ -120,6 120,8 @@ class Next(SimpleNamespace): | |||
| PACKAGE_LOCK = "package-lock.json" | |||
| # Regex to check for message displayed when frontend comes up | |||
| FRONTEND_LISTENING_REGEX = "Local:[\\s] (.*)" | |||
| # Place holder domain name | |||
| PLACEHOLDER_DOMAIN = "http://example.com" | |||
There was a problem hiding this comment.
we do not need the placeholder domain if we build the complete sitemap with python
|
|
||
| async def serve_sitemap(state: State, app: App) -> Response: | ||
| sitemap_content = generate_sitemap(app) | ||
| actual_domain = state.router.session.client_token or "http://localhost:3000" |
There was a problem hiding this comment.
iirc you can get the domain with get_config().deploy_url
| def add_sitemap_to_app(app: App): | ||
| @app.add_page(route="/sitemap.xml") | ||
| def sitemap() -> Component: | ||
| return serve_sitemap(State(), app) |
There was a problem hiding this comment.
serve_sitemap returns a fastapi Response not a rx.Component. You shouldn't use @app.add_page to register your fastapi route. Take a look at the docs on how to use fastapi: https://reflex.dev/docs/api-routes/overview/
|
|
||
| # Determine priority based on route depth | ||
| depth = route.count("/") | ||
| priority = max(0.5, 1.0 - (depth * 0.1)) |
There was a problem hiding this comment.
I like the idea of an auto priority detection. Maybe we could add an argument for explicit priority, changefreq etc. to app.add_page and/or the @rx.page decorator
There was a problem hiding this comment.
@benedikt-bartscher thanks for the feedack!
…eflex-dev#3839) * guess_type: if the type is optional, treat it like it's "not None" When guessing the type for an Optional annotation, the Optional was already being stripped off, but this value was being ignored, except for error messages. So actually use the Optional-stripped value. * Strip Optional when casting Var .to
If the user provided an `appearance` or `color_mode` value to `rx.theme`, then the resulting value ended up being double-double-quoted in the resulting JS output. Instead remove the quotes from the context.js.jinja2 and always pass appearance as a Var.
… stuck with httpx.get SSL (reflex-dev#3847) * [REF-3633] Introduce a workaround for enterprise users who get stuck with httpx.get SSL Setting SSL_NO_VERIFY=1 will disable SSL verification during `reflex init` * Also install fnm using `reflex.utils.net.get`
* Use lru_cache on expensive typing-related functions * Skip instantiation of VarData unless data was actually merged * Revert "integration: bump listening timeout to 1800 seconds" This reverts commit a94eedf. * Revert "integration: bump listening timeout to 1200 seconds" This reverts commit 86563b6.
* add failing test * catch unhandled NotImplementedError
* Remove deprecations * remove prop conversion * fix tests * fix slight issue * fix darglint
…t match spec (reflex-dev#3853) * test_component: improve valid/invalid event trigger tests Add test cases for event triggers defined as annotations. Add additional cases around lambda returning different values. Improve assertions for invalid tests (each line needs its own `pytest.raises`). More invalid test cases. * [REF-3589] raise EventHandlerArgMismatch when event handler args don't match spec Improve error message for common issue. Previously when the event handler arguments didn't match the spec, the traceback resulted in: ``` OSError: could not get source code ``` Now this problem is traceable as a distinct error condition and users are empowered to debug their code and reference the documentation (to be updated) for further information. * raise EventFnArgMismatch when lambda args don't match event trigger spec Improve error message for another common issue encountered in the reflex framework. Previous error message was ``` TypeError: index.<locals>.<lambda>() takes 0 positional arguments but 1 was given ``` * Fix up lambda test cases * call_event_fn: adjust number of args for bound methods
* implement disk state manager * put states inside of a folder * return root state all the time * factor out code * add docs for token expiration * cache states directory * call absolute on web directory * change dir to app path when testing the backend * remove accidental 🥒 * test disk for now * modify schema * only serialize specific stuff * fix issue in types * what is a kilometer * create folder if it doesn't exist in write * this code hates me * check if the file isn't empty * add try except clause * add check for directory again
…ev#3841) * add var_operations and move some operations to the new style * change bound style * can't assume int anymore * slice is not hashable (how did this work bef) * convert to int explicitly * move the rest of the operations to new style * fix bool guess type * forgot to precommit dangit * add type ignore to bool for now
* Added API Endpoint * Added API Endpoint * Added Unit Tests * Added Unit Tests * main * Apply suggestions from Code Review * Fix Ruff Formatting * Update Socket Events * Async Functions
* Promote `rx.progress` from radix themes * fix pyi * add warning when accessing `rx._x.progress`
When emitting a state update, restore `_self_mutable` to the value it had previously so that `yield` in the middle of `async with self` does not result in an immutable StateProxy. Fix reflex-dev#3869
If a component in the markdown component_map contains children components, use `_get_all_imports` to recursively enumerate them. Fix reflex-dev#3880
…into dynamic_sitemap_gen
9 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
### Type of Change
Hi @benedikt-bartscher, I hope you're well. Apologies for the delay in updating you on this issue. Here is a proposed solution for generating dynamic sitemaps in Reflex apps.
Changes Made
Added a Placeholder Constant:
Nextconstant variables.Updated
next_sitemap.js:siteUrlin the `generImplemented Dynamic Sitemap Generation:
Feel free to let me know if there are any questions or further adjustments needed.