Directive DSL, inline fragments, fragment definitions, custom scalars#116
Open
rellampec wants to merge 33 commits into
Open
Directive DSL, inline fragments, fragment definitions, custom scalars#116rellampec wants to merge 33 commits into
rellampec wants to merge 33 commits into
Conversation
- Client#to_query_string(**kargs, &block): builds full query document from DSL block and returns a String without touching Faraday or HTTP. - Query#spread(fragment_name): emits ...FragmentName in the query string, replacing the deprecated ___Const graphql-client convention. - spec_helper.rb: rescue LoadError on byebug require (Windows Ruby 3.2). - CLAUDE.md: AI agent instructions for this fork.
static_client_query_spec: add schema_path: 'spec/support/fixtures/invoice_api.json'
The client was fetching the schema from the live URL at module load time
(before WebMock is active), causing 403 in CI. Using a local schema file
avoids the network call and matches the pattern in webmock_client_query_spec.
client_dsl_spec: nodes { } → nodes do end (Style/BlockDelimiters)
query.rb: add rubocop:disable Metrics/ClassLength
Class grew past 100 lines after DSL additions (spread, fragment resolution).
spec_helper.rb: add rubocop:disable Lint/HandleExceptions on rescue LoadError
The empty rescue is intentional — byebug unavailable on some platforms.
Note: faraday_adapter_spec.rb:147 fails locally on Windows (TCP error message
format differs from Linux) but passes in CI on ubuntu-latest. Pre-existing.
On Linux, Errno::ECONNREFUSED.new(msg).message prepends 'Connection refused - '. On Windows, the OS prepends a different string. Use a regex that matches the core error_message content rather than the exact platform-specific prefix.
Adds a spec for directive DSL syntax: feeInCents _skip(if: :skip_fee) This should translate to @Skip(if: \) in the GraphQL query. Implementation deferred — tracked in this branch (directives-dsl-support). Base: feature/dsl-mode-only-option.
…ding
README.md:
'Fragment Spreads in the DSL' — documents spread :Name DSL method
(already implemented in feature/dsl-mode-only-option base)
'Fragment Spreads — inline definition' — shows the proposed fragment DSL
(inline fragment(:Name, on: :Type) block, scoped to query call)
marked 'coming in a future release'
'Directives in the DSL' — shows _skip(if:) → @Skip(if: \) proposal
marked 'coming in a future release', anchored to this branch
spec/graphlient/client_query_spec.rb:
Directive test marked pending with explanation — shows intended behaviour
without causing a CI failure while the implementation is outstanding.
74 examples, 0 failures, 1 pending.
…calars
Query refactored into Query (thin shell) + Query::Serializer (composed):
query/directive.rb Directive value object — returned by _name(args),
placed AFTER field name in output (correct order)
query/serializer.rb Main composition; includes all concern modules
query/serializer/scalars.rb SCALAR_TYPES extraction + Serializer.scalar(:sym, Type)
class-level custom scalar registration
query/serializer/arguments.rb hash_arg/args_str/variable_string; field_args/directive_args
split; variable_string bug fixed (lambda case)
query/serializer/evaluator.rb evaluate() via instance_eval
query/serializer/fragments.rb ___Const triple-underscore resolution
query/serializer/directives.rb directive?() predicate; method_missing returns Directive
Removed: query/transcript.rb (empty stub — deleted)
New DSL surface:
_skip(if: :x) → @Skip(if: $x) (any _name directive)
_include(if: :x) → @include(if: $x)
feeInCents _skip(if: :x) → feeInCents @Skip(if: $x)
spread :F, _skip(if: :x) → ...F @Skip(if: $x)
on(:Type) { fields } → ... on Type { fields }
on(:Type, _skip(if: :x)) { } → ... on Type @Skip(if: $x) { }
fragment(:F, on: :T) { } → fragment F on T { } (appended after main query)
Custom scalar registration (client config block):
client = Graphlient::Client.new(url) { |c| c.scalar :date, 'Date' }
query(created_after: :date) { } → query($created_after: Date)
Specs: query_dsl_spec.rb — 16 examples covering all new features
client_query_spec.rb — directive test enabled (was pending, now live)
90 examples total, 0 failures
README.md:
Fragment Spreads and Definitions — remove 'proposed/coming' language,
document fragment(:Name, on: :Type) as shipped, add spread+directive example
Inline Fragments — new section: on(:Type) { } and on(:Type, _skip) { }
Directives in the DSL — rewrite from 'coming soon' to full reference:
on a field, on a spread, on an inline fragment, multiple, no-arg
Custom Scalar Types — new section: c.scalar :date, 'Date' client config
Table of contents updated to match all new sections
spec/graphlient/client_dsl_spec.rb:
to_query_string with directive — verifies @Skip in output
to_query_string with inline fragment — verifies ... on Type
to_query_string with fragment definition — verifies assembled output
scalar registration via client block — verifies c.scalar :date, 'Date'
94 examples, 0 failures
AI agent instructions are local-only and not relevant to the upstream repo. CLAUDE.md is removed from git tracking but kept in the working directory. Added to .gitignore so future sessions don't accidentally commit it.
Root cause: on Ruby 3.1, WebMock's Faraday middleware intercepts requests before they reach Faraday::Adapter::Rack, returning 403 for any URL without a stub. This caused 25 CI failures on that Ruby version. spec/support/context/dummy_client.rb: Replace Faraday::Adapter::Rack with stub_request.to_rack(app). WebMock routes both schema introspection and query execution through the Sinatra dummy app at the WebMock level (before Faraday dispatch). Removed schema_path: so graphql-client gets the full live schema (including test fields: executionErrorInvoice, partialSuccess etc.). spec/graphlient/static_client_query_spec.rb: Same to_rack approach for execution tests (schema_path kept here since the module constants are created at file-load time, before any before block). Removed Faraday::Adapter::Rack from the static client constructor. spec/graphlient/client_schema_spec.rb: Use include instead of eq for error message — Faraday 2.x appends the URL to the message; include matches on both old and new formats. 73 examples, 0 failures
spec/support/context/dummy_client.rb: spec/graphlient/static_client_query_spec.rb: Replace em-dashes (--) with ASCII semicolons/colons in comments. Style/AsciiComments cop requires pure ASCII. Gemfile: Add mutex_m gem -- activesupport 5.x requires mutex_m which was removed from Ruby's standard library in Ruby 3.4. Adding it explicitly silences the warning and fixes the LoadError on Ruby 3.4. .github/workflows/ci.yml: Add Ruby 3.4 to the required CI matrix (was previously only in ruby-head which is allowed to fail). Ruby 3.4 is now released and should be tested.
New sections:
'Build Query Strings without Validation' -- explains the serialization-only
path (no schema load, no graphql-client validation, no HTTP), shows the
basic usage, clarifies when the string can be fed back to client.execute
(fragment-free queries only), explains why fragment spreads cannot go back
through the gem (graphql-client's constant-based fragment model), and
shows how to_query_string is the escape hatch for replacing graphql-client
with your own HTTP transport entirely.
'Fragment Spreads' -- documents Query#spread as the modern alternative to
the triple-underscore convention; covers basic usage and multiple spreads.
Brings in: to_query_string, spread, README documentation for both features (Build Query Strings without Validation + Fragment Spreads sections), CI fixes, gitignore updates. README conflict resolved: directives branch has the fuller Fragment Spreads and Definitions, Inline Fragments, Directives, and Custom Scalars sections -- these supersede the simpler Fragment Spreads section from the base branch.
Adds entries for: directive DSL (_name convention), on(:Type) inline fragments, fragment(:Name, on:) definitions, custom scalar registration, Query::Serializer refactor, Ruby 3.1/3.4 CI fixes.
- All entries single-line matching file convention (no continuation lines) - ashkan18#115 entries restored to their original order and not modified - ashkan18#116 prefix added to all directives/fragments/scalars/CI entries
rellampec
changed the title
Contributor
Author
|
The Danger job is failing due to an expired token in the workflow — unrelated to this PR's changes. |
4 tasks
…cop to ~> 1.0 Removed two inline disable comments (Metrics/ClassLength on Query, Lint/HandleExceptions in spec_helper) in favour of the rubocop_todo.yml approach suggested by upstream maintainer. Bumped rubocop dev dependency from pinned 0.56.0 to ~> 1.0 (required for Ruby 3.2 compatibility — Psych.safe_load signature changed). TargetRubyVersion kept at 2.3 to reflect the gem's minimum supported Ruby. Ran rubocop -a (11 auto-corrections) then --auto-gen-config to regenerate .rubocop_todo.yml with current cop names (Metrics/LineLength renamed to Layout/LineLength, Style/MethodMissingSuper removed, etc.).
Collaborator
Fixed on main, rebase? |
Danger ReportNo issues found. |
Collaborator
|
I merged #115, rebase. Thank you! |
Contributor
Author
|
@dblock fixed rubocop to 1.87.0
aligned with master |
dblock
approved these changes
Jun 11, 2026
dblock
left a comment
dblock
left a comment
Collaborator
There was a problem hiding this comment.
This is excellent work @rellampec, have just one question, lmk what you think?
| client.query do | ||
| query do | ||
| invoice(id: 10) do | ||
| on(:PaidInvoice) do |
Collaborator
There was a problem hiding this comment.
Would we want this to be spread on :PaidInvoice to be consistent?
Contributor
Author
|
Yup... I thought the same thing.
While on is quite explicit, consistency leads to spread with no argument... would read better... although on looks nice.
I can change it
Get Outlook for Android<https://aka.ms/AAb9ysg>
________________________________
From: Daniel (dB.) Doubrovkine ***@***.***>
Sent: Friday, 12 June 2026 01:24:22
To: ashkan18/graphlient ***@***.***>
Cc: rellampec ***@***.***>; Mention ***@***.***>
Subject: Re: [ashkan18/graphlient] Directive DSL, inline fragments, fragment definitions, custom scalars (PR #116)
@dblock approved this pull request.
This is excellent work @rellampec<https://github.com/rellampec>, have just one question, lmk what you think?
________________________________
In README.md<#116 (comment)>:
query do
invoice(id: 10) do
+ on(:PaidInvoice) do
Would we want this to be spread on :PaidInvoice to be consistent?
—
Reply to this email directly, view it on GitHub<#116?email_source=notifications&email_token=AHHYXLHKTTL3YPTTNF4TTAD47M5SLA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINBYGEZDMOBQGQYKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#pullrequestreview-4481268040>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AHHYXLDJGLUHBCDOXXL3CZT47M5SLAVCNFSNUABFKJSXA33TNF2G64TZHMYTANJXGA4TIOJWHNEXG43VMU5TINRTGA2DSMJZGM32C5QC>.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS<https://github.com/notifications/mobile/ios/AHHYXLF36XE7N6VSTQIZMIT47M5SLA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINBYGEZDMOBQGQYKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y> and Android<https://github.com/notifications/mobile/android/AHHYXLHZIZHAVEGBDTY5ZAD47M5SLA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINBYGEZDMOBQGQYKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>. Download it today!
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Contributor
Author
|
Another thing word reviewed, that I think slipped through my finger, is if we should use query_to_string instead. #query will go through the graphql-client parsing, and would require the constant to be declaren. Will have a look later today.
Get Outlook for Android<https://aka.ms/AAb9ysg>
________________________________
From: Daniel (dB.) Doubrovkine ***@***.***>
Sent: Friday, 12 June 2026 01:24:22
To: ashkan18/graphlient ***@***.***>
Cc: rellampec ***@***.***>; Mention ***@***.***>
Subject: Re: [ashkan18/graphlient] Directive DSL, inline fragments, fragment definitions, custom scalars (PR #116)
@dblock approved this pull request.
This is excellent work @rellampec<https://github.com/rellampec>, have just one question, lmk what you think?
________________________________
In README.md<#116 (comment)>:
query do
invoice(id: 10) do
+ on(:PaidInvoice) do
Would we want this to be spread on :PaidInvoice to be consistent?
—
Reply to this email directly, view it on GitHub<#116?email_source=notifications&email_token=AHHYXLHKTTL3YPTTNF4TTAD47M5SLA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINBYGEZDMOBQGQYKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#pullrequestreview-4481268040>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AHHYXLDJGLUHBCDOXXL3CZT47M5SLAVCNFSNUABFKJSXA33TNF2G64TZHMYTANJXGA4TIOJWHNEXG43VMU5TINRTGA2DSMJZGM32C5QC>.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS<https://github.com/notifications/mobile/ios/AHHYXLF36XE7N6VSTQIZMIT47M5SLA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINBYGEZDMOBQGQYKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y> and Android<https://github.com/notifications/mobile/android/AHHYXLHZIZHAVEGBDTY5ZAD47M5SLA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINBYGEZDMOBQGQYKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>. Download it today!
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Collaborator
|
@rellampec lmk when it's ready for a re-review, thank you! Also you seem to know what you're doing :) @ashkan18 and I would love to add you to (co)maintainers of this gem, drop me an email to dblock[at]dblock[dot]org if you're interested, including your rubygems email/username so you can maybe make the next release |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Directive DSL, inline fragments, fragment definitions, custom scalars
Depends on #115.
What
Extends the DSL block API with the full GraphQL directive and fragment
surface, and adds custom scalar type registration.
Directive syntax — any
_namemethod becomes a@namedirective:on(:TypeName)— inline fragments for union/interface types.fragment(:Name, on: :Type)— defines a fragment inline and appends itto the query document automatically. No global registry.
c.scalar :date, 'Date'— register custom scalar types alongside thebuilt-in
:int,:float,:string,:boolean.Changes
lib/graphlient/query/directive.rb—Directivevalue objectlib/graphlient/query/serializer.rb+ concern modules —Queryrefactoredlib/graphlient/client.rb—scalarregistration methodspec/graphlient/query_dsl_spec.rb— new spec file (16 examples)README.md— Directives, Inline Fragments, Fragment Spreads and Definitions, Custom ScalarsChecklist