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

ordered result example #1

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

ordered result example #1

wants to merge 3 commits into from

Conversation

benwilson512
Copy link
Contributor

@benwilson512 benwilson512 commented Nov 16, 2017

Some things to note about this:

  • I definitely want to make this easier for folks, in that they shouldn't need to copy a ~100 line file into their project just to change on line. This is also brittle, since it won't be easy to keep up with any changes Absinthe makes. See the module notes for ideas.
  • There isn't any need to change middleware at all, or mess with the default middleware. Maintaining order has nothing to do with middleware
  • Pipeline configuration is more of a pain than it should be. We should make that easier too.
  • This avoids global config flags. This is good because concerns like this are rather client specific. An OrdMap or :orddict or whatever it is you end up using won't be as ergonomic to work with in Elixir itself as a map, so this preserves the normal Absinthe.run behaviour.

end

defp field_data(fields, errors, acc \\ [])
defp field_data([], errors, acc), do: {{:lists.reverse(acc)}, errors}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line here in the normal result phase is:

defp field_data([], errors, acc), do: {Map.new(acc), errors}

This is the only line in the entire file that is different.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main challenge here is that for large results this function gets called a LOT, and so I want to avoid dynamic lookup. One idea is to use macros:

defmodule BlogWeb.OrdGraphQLResult do
  use Absinthe.ResultPhase
  
  def to_object_result(acc) do
    {:lists.reverse(acc)}
    # or you could do `OrdMap.new(:lists.reverse(acc))`
  end
end

pipeline_opts = Keyword.put(pipeline_opts, :result_phase, BlogWeb.OrdGraphQLResult)
config.schema_mod
|> Absinthe.Pipeline.for_document(pipeline_opts)
|> Absinthe.Pipeline.replace(Absinthe.Phase.Document.Result, BlogWeb.OrdGraphQLResult)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Configuration here is just slightly more complicated than we want.

@MartinKavik
Copy link

Hey @benwilson512, I spent today's afternoon and evening trying to install this project's dependencies and run a server, but I wasn't lucky.

I encountered many platform specific (Windows) and maybe general problems, e.g. with:

Well, I've managed to compile Elixir 1.6.0-dev from source code - it solved some issues and I've updated this guide: https://github.com/elixir-lang/elixir/wiki/Windows.
However, it's recommended to install OTP 21 to eliminate the problem with jiffy/rebar3, but compilation OTP from sources is very painful (at least for Windows users).

It's not possible for me to switch to another platform now, so I have to wait for official OTP 21 release and then I'll try it again. Sorry for that and thanks for your time and this PR.
(I'm new to Elixir world, because I'm mainly Typescript/JS/C# programmer in day-job, so if you know another way to start this project I will be very grateful and I can write some guides about it later.)

@benwilson512
Copy link
Contributor Author

Hey! Sorry about the difficulties, I'll get a PR up that shows the main point I'm going for without the challenging libraries you have mentioned.

@benwilson512
Copy link
Contributor Author

Should go up sometime tomorrow.

@MartinKavik
Copy link

MartinKavik commented Nov 19, 2017

Hey! Success! I've managed to run this with Nanobox and Docker.
So everything works fine, except GraphiQL, I suggest to add codec and pipeline settings:

forward "/graphiql", Absinthe.Plug.GraphiQL,
      schema: BlogWeb.Schema,
      json_codec: BlogWeb.JSON,
      pipeline: {__MODULE__, :absinthe_pipeline}

and add flag to json.ex (see #2):

:jiffy.encode(data, [:use_nil])

I think a guide about running absinthe_tutorial with Nanobox will be useful. And a guide with Nanobox and Elm frontend for absinthe_tutorial as well.
@bruce What do you think about it? And do you prefer Elm FE with basic HTML or only writing GraphQL responses to console.log?

Martin Kavík and others added 2 commits November 19, 2017 20:51
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.

2 participants