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

Subscription prime bugfixes #1

Open
wants to merge 4 commits into
base: subscription-prime
Choose a base branch
from

Conversation

AdSkipper1337
Copy link

@AdSkipper1337 AdSkipper1337 commented Dec 21, 2022

Hey, I checked out your branch!

With the two changes that I made it works good for me - though the second change is an issue that probably creates more problems then it solves.

The first fix is just a trivial fix that handles an oversight: In absinthe you can have a GraphQL query that returns a single element instead of a list, the way the code worked before it was assuming that its a list, just added the case and that fixed the problem.

The second thing is that I commented out the line in get_ordinal because it causes problems for me.
And since I do not use the ordinal feature I can not properly test it for you so I will refrain from changing anything here beyond disabling the feature.
The reason why this causes a problem is that I (and I assume many others) have side effects in their subscription config function.
And this causes it to be called multiple times. This breaks my entire app and will probably have to be
done differently unless you want the library to have breaking changes for many users. Probably it can be solved by passing the ordinal callback to the options in the same way you handle it for prime, though it seems to me ordinal is handled in a bit more complex way then prime, and is dispatched in ways that I would need some time to understand, which is why I refrained from changing the logic and just commented it out to make the branch work for my project.

Other then that it is great and does what its supposed to do, if other issues arise I will report.

Best

@AdSkipper1337
Copy link
Author

AdSkipper1337 commented Dec 23, 2022

Sorry seems like I misunderstood what prime was doing, it is apparently much more specific then I understood it to be.
I thought it was just a generic callback which sends the result to the api, but seems like you do something else with the continuation... I changed in on my fork to now just do what I want it to do, but this is definitely not the intent of your feature (The issue I ran into is that I want it to also send back empty results to the frontend. Effectively I just want it as a replacement for query and subscribe to just be subscribe - among other things which are actually not possible without this or extremely inconveniernt). Do you have an idea how I could get my desired functionality without crashing your setup? Or maybe you can explain to me your intent, perhaps it is superior to what I want but I just do not understand it.
It seems to me the feature is very specific to your application and would be incompatible to what I am making and is therefore not generic enough, but perhaps I am wrong

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.

1 participant