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.

Sign up for GitHub

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

Jump to bottom

Remove Addressable monkey patch? #49

Open
dentarg opened this issue Apr 19, 2023 · 3 comments
Open

Remove Addressable monkey patch? #49

dentarg opened this issue Apr 19, 2023 · 3 comments

Comments

@dentarg
Copy link

dentarg commented Apr 19, 2023

From Addressable 2.8.2 it no longer works:

$ ruby -rbundler/inline -e 'gemfile do; source "https://rubygems.org"; gem "postrank-uri"; gem "addressable", "2.8.2"; end; Addressable::URI.parse("google.com").normalize'
/Users/dentarg/.arm64_rubies/3.2.2/lib/ruby/gems/3.2.0/gems/addressable-2.8.2/lib/addressable/uri.rb:1636:in `query=': Can't convert Object into String. (TypeError)

        raise TypeError, "Can't convert #{new_query.class} into String."
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
	from /Users/dentarg/.arm64_rubies/3.2.2/lib/ruby/gems/3.2.0/gems/addressable-2.8.2/lib/addressable/uri.rb:851:in `block in initialize'
	from /Users/dentarg/.arm64_rubies/3.2.2/lib/ruby/gems/3.2.0/gems/addressable-2.8.2/lib/addressable/uri.rb:2392:in `defer_validation'
	from /Users/dentarg/.arm64_rubies/3.2.2/lib/ruby/gems/3.2.0/gems/addressable-2.8.2/lib/addressable/uri.rb:840:in `initialize'
	from /Users/dentarg/.arm64_rubies/3.2.2/lib/ruby/gems/3.2.0/gems/addressable-2.8.2/lib/addressable/uri.rb:2168:in `new'
	from /Users/dentarg/.arm64_rubies/3.2.2/lib/ruby/gems/3.2.0/gems/addressable-2.8.2/lib/addressable/uri.rb:2168:in `normalize'
	from -e:1:in `<main>'

Seeing the patch is soon 12 years old, 110ed0b, is it still needed?

Was originally reported in sporkmonger/addressable#506
@dentarg dentarg mentioned this issue Apr 19, 2023
Closed
@igrigorik
Copy link
Member

Yes, probably due for an update! Would you be willing to stage a PR?

@dentarg
Copy link
Author

dentarg commented Jul 18, 2023 •
edited

I'm not using postrank but maybe @danielnolan or @rickyc or @ck250186 is up for it! (users that reported the problem in the first place)

vasconsaurus added a commit to meedan/pender that referenced this issue Jul 18, 2023
@vasconsaurus
downgrade addressable to 2.8.1
6af1098 
There seems to be a super old monkey patch in postrank that messes
with the normalize method, starting at 2.8.2 version. Until postrank
fixes that we can only go up to 2.8.1

relevant links:
sporkmonger/addressable#513
sporkmonger/addressable#506
postrank-labs/postrank-uri#49
@vasconsaurus vasconsaurus mentioned this issue Jul 18, 2023
Merged
vasconsaurus added a commit to meedan/pender that referenced this issue Jul 19, 2023
@vasconsaurus
3384 – update rails to ~> 6.1.7 (#365)
e82abb4 
* update rails to 6.1.7.4 while keeping sidekiq under 7

delayed extensions was removed in sidekiq 7 and that breaks things
for us
https://github.com/sidekiq/sidekiq/blob/main/Changes.md#640

* downgrade addressable to 2.8.1

There seems to be a super old monkey patch in postrank that messes
with the normalize method, starting at 2.8.2 version. Until postrank
fixes that we can only go up to 2.8.1

relevant links:
sporkmonger/addressable#513
sporkmonger/addressable#506
postrank-labs/postrank-uri#49
@ericfirth ericfirth mentioned this issue Jan 4, 2024
Open
@ericfirth
Copy link

I am running into this issue and its making difficult to update a bunch of gems, so I attempted a fix here: #50

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@igrigorik @dentarg @ericfirth