Skip to content

Centralize protobuf scalar initialization#1313

Merged
jwils merged 1 commit into
mainfrom
joshuaw/proto-scalar-initialization
Jul 20, 2026
Merged

Centralize protobuf scalar initialization#1313
jwils merged 1 commit into
mainfrom
joshuaw/proto-scalar-initialization

Conversation

@jwils

@jwils jwils commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Why

Built-in scalar defaults and final protobuf validation are split between the factory and scalar extension.

What

  • Centralize built-in protobuf mappings and initialization in ScalarTypeExtension.
  • Keep the factory responsible only for extending the scalar and forwarding its configuration block.
  • Preserve the existing default, user configuration, and validation order, including renamed built-in scalars.

Validation

  • script/run_gem_specs elasticgraph-proto_ingestion
  • script/lint elasticgraph-proto_ingestion
  • script/type_check

Risk assessment

Low. This is a structural refactor that preserves existing scalar configuration behavior.

References

@myronmarston myronmarston left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you apply this same pattern to elasticgraph-json_ingestion? It could be improved in the same way:

BUILT_IN_SCALAR_JSON_SCHEMA_OPTIONS_BY_NAME = {
"Boolean" => {type: "boolean"},
"Float" => {type: "number"},
"ID" => {type: "string"},
"Int" => {type: "integer", minimum: INT_MIN, maximum: INT_MAX},
"String" => {type: "string"},
"Cursor" => {type: "string"},
"Date" => {type: "string", format: "date"},
"DateTime" => {type: "string", format: "date-time"},
"LocalTime" => {type: "string", pattern: VALID_LOCAL_TIME_JSON_SCHEMA_PATTERN},
"TimeZone" => {type: "string", enum: GraphQL::ScalarCoercionAdapters::VALID_TIME_ZONES.to_a.freeze},
"Untyped" => {type: ["array", "boolean", "integer", "number", "object", "string"].freeze},
"JsonSafeLong" => {type: "integer", minimum: JSON_SAFE_LONG_MIN, maximum: JSON_SAFE_LONG_MAX},
"LongString" => {type: "integer", minimum: LONG_STRING_MIN, maximum: LONG_STRING_MAX}
}.freeze

@jwils
jwils marked this pull request as ready for review July 20, 2026 19:59
@jwils
jwils merged commit 669b770 into main Jul 20, 2026
24 checks passed
@jwils
jwils deleted the joshuaw/proto-scalar-initialization branch July 20, 2026 19:59
jwils added a commit that referenced this pull request Jul 21, 2026
## Why

Built-in JSON scalar defaults and final JSON Schema validation are split
between the factory and scalar extension.

## What

- Centralize built-in JSON Schema mappings and initialization in
`ScalarTypeExtension`.
- Keep the factory responsible only for extending the scalar and
forwarding its configuration block.
- Preserve the existing default, user configuration, runtime metadata,
and validation order, including renamed built-in scalars.

## Validation

- `script/run_gem_specs elasticgraph-json_ingestion`
- `script/lint elasticgraph-json_ingestion`
- `script/type_check`

## Risk assessment

Low. This is a structural refactor that preserves existing JSON scalar
configuration behavior.

## References

-
#1313 (review)
- Follow-up to #1313
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