Skip to content

gh-148849: Deprecate http.cookies.BaseCookie.js_output()#148978

Merged
serhiy-storchaka merged 14 commits intopython:mainfrom
kishorhange111:gh-148849-deprecate-js-output
May 4, 2026
Merged

gh-148849: Deprecate http.cookies.BaseCookie.js_output()#148978
serhiy-storchaka merged 14 commits intopython:mainfrom
kishorhange111:gh-148849-deprecate-js-output

Conversation

@kishorhange111
Copy link
Copy Markdown
Contributor

@kishorhange111 kishorhange111 commented Apr 25, 2026

gh-148849: Deprecate http.cookies.BaseCookie.js_output()

I didn’t see anyone assigned to this issue, so I tried working on it. I’m new to open source contributions, so please let me know if anything should be done differently.

This deprecates BaseCookie.js_output() as discussed in the issue. The method seems to be unused and tied to older browser behavior that isn’t really relevant anymore.

I added a deprecation decorator to the method, updated the existing tests to expect a DeprecationWarning, and updated the docs along with a NEWS entry.

I ran test_http_cookies locally — all tests pass (33 tests, 1 skipped):

Ran 33 tests in 0.122s
OK (skipped=1)

I also checked that calling js_output() raises the warning while still returning the same result.

I left Morsel.js_output() unchanged since the issue only mentions BaseCookie.js_output(), but I can update that too if needed.

Would appreciate a review whenever you get a chance. Thanks!

Fixes gh-148849


📚 Documentation preview 📚: https://cpython-previews--148978.org.readthedocs.build/

@python-cla-bot
Copy link
Copy Markdown

python-cla-bot Bot commented Apr 25, 2026

All commit authors signed the Contributor License Agreement.

CLA signed

Comment thread Doc/library/http.cookies.rst Outdated
Comment thread Doc/library/http.cookies.rst Outdated
Comment thread Doc/whatsnew/3.15.rst Outdated
Comment thread Doc/whatsnew/3.15.rst Outdated
Comment thread Lib/http/cookies.py Outdated
Comment thread Lib/test/test_http_cookies.py Outdated
Comment thread Lib/test/test_http_cookies.py Outdated
Comment thread Lib/test/test_http_cookies.py Outdated
@bedevere-app
Copy link
Copy Markdown

bedevere-app Bot commented Apr 25, 2026

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@picnixz
Copy link
Copy Markdown
Member

picnixz commented Apr 25, 2026

Please, do not update your branch if there is no need to. Please read https://devguide.python.org/getting-started/pull-request-lifecycle/ before doing future contributions.

Comment thread Doc/library/http.cookies.rst Outdated
Comment thread Misc/NEWS.d/next/Library/2026-04-25-12-04-27.gh-issue-148849.Vk6yEW.rst Outdated
@kishorhange111 kishorhange111 force-pushed the gh-148849-deprecate-js-output branch from d9ae235 to d1e4a57 Compare April 25, 2026 08:26
@kishorhange111
Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again

@bedevere-app
Copy link
Copy Markdown

bedevere-app Bot commented Apr 25, 2026

Thanks for making the requested changes!

@picnixz: please review the changes made to this pull request.

@bedevere-app bedevere-app Bot requested a review from picnixz April 25, 2026 08:58
@kishorhange111
Copy link
Copy Markdown
Contributor Author

Please let me know if there are any further suggestions or changes needed.

Comment thread Doc/library/http.cookies.rst Outdated
Co-authored-by: Stan Ulbrych <stan@python.org>
Comment thread Doc/whatsnew/3.15.rst Outdated
Comment thread Lib/http/cookies.py Outdated
Comment thread Lib/test/test_http_cookies.py Outdated
kishorhange111 and others added 2 commits April 25, 2026 15:09
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@kishorhange111
Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again

@bedevere-app
Copy link
Copy Markdown

bedevere-app Bot commented Apr 25, 2026

Thanks for making the requested changes!

@picnixz: please review the changes made to this pull request.

Comment thread Lib/http/cookies.py
Comment thread Lib/http/cookies.py Outdated
@picnixz
Copy link
Copy Markdown
Member

picnixz commented Apr 27, 2026

Nothing to do on your side! Don't update your branch since there won't be a need to, I'll just wait a few days before alpha closes to merge this.

cc @sethmlarson @serhiy-storchaka @encukou

Copy link
Copy Markdown
Contributor

@sethmlarson sethmlarson left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM. 👍

@encukou
Copy link
Copy Markdown
Member

encukou commented May 1, 2026

Sorry for being late to the party, but, IMO Morsel.js_output should be deprecated as well -- it has exactly the same issues.
(and BaseCookie uses that, so the warning should only be emitted there.)

@kishorhange111
Copy link
Copy Markdown
Contributor Author

Hey, quick question — should I include this change in the current PR, or would you prefer I open a separate PR specifically for the Morsel.js_output update?

@encukou
Copy link
Copy Markdown
Member

encukou commented May 1, 2026

I'd prefer here since they're so related.

@kishorhange111
Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again.

As suggested by @encukou, I’ve deprecated Morsel.js_output(). The implementation now routes through Morsel.js_output(), so the deprecation warning is emitted only once.

Note on stacklevel:
The current implementation uses warnings._deprecated(), which has a fixed stacklevel=3. As a result, when BaseCookie.js_output() calls Morsel.js_output(), the warning points to BaseCookie.js_output() rather than the user's code. This could be improved by switching to warnings.warn() with a configurable stacklevel.
I'm happy to adjust this if preferred .

@bedevere-app
Copy link
Copy Markdown

bedevere-app Bot commented May 2, 2026

Thanks for making the requested changes!

@picnixz, @StanFromIreland, @serhiy-storchaka: please review the changes made to this pull request.

@read-the-docs-community
Copy link
Copy Markdown

read-the-docs-community Bot commented May 2, 2026

Documentation build overview

📚 cpython-previews | 🛠️ Build #32507822 | 📁 Comparing a91d066 against main (db0ee44)

  🔍 Preview build  

40 files changed · ± 40 modified

± Modified

Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

To avoid multiple deprecation warnings from internal use, you can add private Morsel._js_output and use it in BaseCookie.js_output and Morsel.js_output after emitting a warning.

You can use self.assertEqual(cm.filename, __file__) to assert that the warning points to the correct calling place.

@kishorhange111
Copy link
Copy Markdown
Contributor Author

@serhiy-storchaka Thank you so much for the suggestion!
I've added Morsel._js_output() as a protected method containing the actual logic, so both Morsel.js_output() and BaseCookie.js_output() can warn independently and then delegate to _js_output(). This cleanly avoids multiple warnings and also fixes the stacklevel issue. I've also added cm.filename == file assertions to verify that the warning correctly points to the user's call site.

Copy link
Copy Markdown
Contributor

@sethmlarson sethmlarson left a comment

Choose a reason for hiding this comment

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

Thank you!

@serhiy-storchaka serhiy-storchaka merged commit 246fe14 into python:main May 4, 2026
52 of 53 checks passed
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.

Deprecate BaseCookie.js_output() method

6 participants