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

Move requests (required for SSR only) to an extra #73

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

akx
Copy link
Contributor

@akx akx commented Mar 17, 2025

Since only SSR support requires requests, it shouldn't be installed (along with its transitive dependencies) for projects that don't require SSR.

@BrandonShar
Copy link
Collaborator

I'm somewhat torn on this one because I really value ease of setup, but I would guess that SSR is a significant minority of new installs; I've personally never used it.

I'm strongly leaning towards merging this, but I'd be interested in some more opinions before I do.

@svengt what do you think?

@akx akx force-pushed the optional-requests branch from 40f18da to 3140660 Compare March 26, 2025 07:07
@svengt
Copy link
Contributor

svengt commented Mar 26, 2025

@BrandonShar @akx I agree this dependency could be marked as optional. However, I'm hesitant about making this change now since it would be breaking - anyone currently relying on inertia-django to handle the requests dependency would need to either install it themselves or add the extra flag to their requirements.

My suggestion would be to leave this as is until we're planning other breaking changes, so we can bundle them together in a proper major version update.

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.

3 participants