ueberauth / ueberauth_twitter

Twitter Strategy for Überauth
MIT License
36 stars 38 forks source link

Ensuring can use library with Jason #26

Closed snewcomer closed 3 years ago

snewcomer commented 5 years ago

Currently, without Poison installed, decoding the response will fail. With the introduction of Jason, we could allow the consumer to specify their own json parser. What do you think? If you agree, I'll put up a small PR.

One idea I have via a config option:

config :ueberauth_twitter, json_library: Jason

doomspork commented 5 years ago

@snewcomer I think this is a good idea and should be supported not just by this strategy but all of them. What do you think about that?

@hassox / @yordis we've talked about this a little, want to weigh in?

yordis commented 5 years ago

@doomspork this will be an issue, the best we do is exactly what @snewcomer propose.

Do we want to read the encoder from Ueberauth instead of the strategies?

If we add this to every single strategy will be hard to maintain for us, also for the end users since they would need to change it per strategy.

I would prefer to put json_library/0 in Ueberauth package and use it from the strategies. But I understand that it is a tradeoff since some people could ask for changing it only in specific strategies but at that moment I think they are not doing the right thing.

chulkilee commented 3 years ago

Seems like now you can use https://hexdocs.pm/ueberauth/0.6.3/Ueberauth.html#json_library/1

https://github.com/ueberauth/ueberauth/pull/88

yordis commented 3 years ago

Yep https://github.com/ueberauth/ueberauth_twitter/blob/620f476fb8d12a05de004936662c0b8fb4ce8785/lib/ueberauth/strategy/twitter/internal.ex#L31