Add support of initializing the extension on a Blueprint #25
Conversation
Adds support for initializing the extension on a `Blueprint`.
flask_inertia/inertia.py
Outdated
| # TODO: maybe `Scaffold` instead? | ||
| FlaskOrBlueprint = Union[Flask, Blueprint] |
There was a problem hiding this comment.
Would you prefer Scaffold instead of this union type? Not sure what the implications are.
There was a problem hiding this comment.
Since Python 3.10 typing supports and recommends the shorthand Flask | Blueprint . I think you can use this since it has been released for 3 years now. Also I'm not really sure you need to declare it as a new type. I find the following notation totally fine
def __init__(self, app: Optional[Flask | Blueprint] = None):There was a problem hiding this comment.
Wasn't sure what your minimum supported python version is, which is why I went with Union. Technically python 3.9 is not end-of-life yet but up to you if you're supporting it.
flask_inertia/inertia.py
Outdated
| # TODO: support multiple instances of Inertia, each on different | ||
| # blueprints? Possibly by using blueprint's name as part of the | ||
| # key in app.extensions. e.g: | ||
| # app.extensions[f"inertia.{state.blueprint.name}"] = self | ||
| self._init_extension(state.app) |
There was a problem hiding this comment.
What are your thoughts on this? Not sure if there is a valid use-case for multiple Inertia instances on multiple blueprints.
The way I have it now, it would silently break in that case. Each blueprint registration would clobber the previous.
There was a problem hiding this comment.
Or if you can find a way to store a reference to the Inertia instance on the blueprint, that would be better. I didn't see any in the flask docs though. There is no extensions on Blueprints.
There was a problem hiding this comment.
The only reason I would see using this extension on a Blueprint instead of on the current app is a migration plan since you can use the extension alongside a normal Flask app. You don't need to support multiple extension instances.
| class TestBlueprint(unittest.TestCase): | ||
| """Flask-Inertia blueprint tests.""" |
There was a problem hiding this comment.
I added a new test class (and test file), but if you'd prefer a different setup let me know. There was some copy/pasting from test_app.py.
There was a problem hiding this comment.
I'm not against adding a new file to tests Blueprint but I would prefer to move the tests setup in the __init__ file instead of copying it.
There was a problem hiding this comment.
Do you mean making a base class in __init__.py which TestInertia and TestBlueprint would inherit from?
There was a problem hiding this comment.
Not necessarily a base class but maybe all the views and the config class to avoid copy/pasting.
flask_inertia/inertia.py
Outdated
| # TODO: maybe `Scaffold` instead? | ||
| FlaskOrBlueprint = Union[Flask, Blueprint] |
There was a problem hiding this comment.
Since Python 3.10 typing supports and recommends the shorthand Flask | Blueprint . I think you can use this since it has been released for 3 years now. Also I'm not really sure you need to declare it as a new type. I find the following notation totally fine
def __init__(self, app: Optional[Flask | Blueprint] = None):
flask_inertia/inertia.py
Outdated
| # TODO: support multiple instances of Inertia, each on different | ||
| # blueprints? Possibly by using blueprint's name as part of the | ||
| # key in app.extensions. e.g: | ||
| # app.extensions[f"inertia.{state.blueprint.name}"] = self | ||
| self._init_extension(state.app) |
There was a problem hiding this comment.
The only reason I would see using this extension on a Blueprint instead of on the current app is a migration plan since you can use the extension alongside a normal Flask app. You don't need to support multiple extension instances.
| class TestBlueprint(unittest.TestCase): | ||
| """Flask-Inertia blueprint tests.""" |
There was a problem hiding this comment.
I'm not against adding a new file to tests Blueprint but I would prefer to move the tests setup in the __init__ file instead of copying it.
flask_inertia/inertia.py
Outdated
| from flask import Flask, Response, current_app, request | ||
| from flask import Blueprint, Flask, Response, current_app, request | ||
| from flask.sansio.app import App | ||
| from flask.sansio.blueprints import BlueprintSetupState |
There was a problem hiding this comment.
Why do you get this type in sansio instead of dedicated module ?
| from flask.sansio.blueprints import BlueprintSetupState | |
| from flask.blueprints import BlueprintSetupState |
remove sansio from import path
Closes #24
Adds support for initializing the extension on a
Blueprint.Work in progress, wanted to get your input on a few things.