Add `include_undefined_query_parameters` config flag by jbaxendale-ut · Pull Request #2 · user-interviews/js-routes

jbaxendale-ut · 2026-05-27T20:37:05Z

JavaScript uses undefined to mean “not provided” but js-routes was treating undefined query object values the same as null, which ultimately became present and nil when parsed by Rails 8.1's new parameter handling (where they came in as ?foo as opposed to previous ?foo= to avoid an exploit vector)

This PR adds an opt-in configuration setting, include_undefined_query_parameters, that makes the default serializer skip object properties whose value is undefined.

Explicit null still means Rails nil and keeps the Rails-version-specific behavior, including Rails 8.1 bare-key serialization.

The nil value of the flag will prompt those upgrading to see a warning about choosing a deliberate behavior and leave the new behavior disabled, while the generator will default new installations to false

Finally, updated a few version checks to use Gem::Version which are a bit safer than straight string comparisons (as I ran into issues testing from a few of these not resolving completely at the time), as well as adds Rails 8.1 and Ruby 3.4 to the CI matrix.

Mirroring in the main repo: railsware#346

^{Need help on this PR? Tag @codesmith with what you need. Autofix is disabled.}

JavaScript uses undefined to mean “not provided,” but js-routes was treating undefined query object values the same as Rails nil. After Rails 8.1, that became more visible because nil query params serialize as bare keys, so an omitted JS value could unexpectedly produce a query param like ?foo. Adds an opt-in js-routes setting, omit_undefined_query_parameters, that makes the default serializer skip object properties whose value is undefined. Explicit null still means Rails nil and keeps the Rails-version-specific behavior, including Rails 8.1 bare-key serialization.

Copilot

Pull request overview

Adds an opt-in configuration flag to the generated js-routes runtime so that the built-in query serializer can omit object properties whose value is undefined (while preserving existing Rails-nil behavior for explicit null, including Rails 8.1 bare-key serialization).

Changes:

Introduces omit_undefined_query_parameters configuration (Ruby config → JS runtime RubyVariables → runtime Configuration).
Updates the built-in default serializer to skip undefined object properties when the flag is enabled.
Adds/updates specs, typings, and README documentation to cover and expose the new option.

Reviewed changes

Copilot reviewed 8 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
spec/js_routes/rails_routes_compatibility_spec.rb	Adds coverage ensuring optional path fragment handling isn’t broken when the new flag is enabled.
spec/js_routes/options_spec.rb	Verifies the new config is exposed via `Routes.config()`.
spec/js_routes/module_types/dts/routes.spec.d.ts	Updates DTS spec fixture to include the new configuration and Ruby variable types.
spec/js_routes/default_serializer_spec.rb	Adds detailed serializer behavior coverage for default vs. omit-enabled modes across Rails nil serialization variants.
Readme.md	Documents the new `omit_undefined_query_parameters` option and its interaction with Rails nil behavior and custom serializers.
lib/routes.ts	Adds the config field, plumbs it from `RubyVariables`, and updates the default serializer to omit `undefined` object props when enabled.
lib/routes.js	Compiled runtime update mirroring the TypeScript changes.
lib/routes.d.ts	Updates published TypeScript declarations for the new config/Ruby variable fields.
lib/js_routes/instance.rb	Exposes `OMIT_UNDEFINED_QUERY_PARAMETERS` into the generated JS variables.
lib/js_routes/configuration.rb	Adds the Ruby-side configuration attribute with default `false`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

jbaxendale-ut · 2026-05-27T20:44:06Z

+    }
  end

-  it "should provide this method" do


this was duplicated on line 15

… changing the default behavior. This patch implements that. What changed: - omit_undefined_query_parameters is now a tri-state migration option: - nil default: keeps legacy behavior and warns - false: keeps legacy behavior without warning - true: omits object keys whose value is undefined - Existing apps remain backward compatible because the unset default still serializes undefined as Rails nil. - New generated initializers opt into the future behavior: c.omit_undefined_query_parameters = true - The generated JS config now represents the unset Ruby value as null, and TypeScript/DTS types allow boolean | null. - Tests cover the migration path: - unset warns and preserves legacy serialization - explicit false preserves legacy serialization without warning - explicit true omits undefined - generator template sets the option to true This matches the requested “nil by default, backward compatible with warning, generator sets true” first phase.

Review asked that Ruby make the migration decision before generation and that the TypeScript runtime receive a non-nullable boolean. Emit true only for an explicit true setting and update generated DTS fixtures to match.

Review asked us to avoid migration warnings in regular tests by specifying the new config. Default evallib and the suite-wide JsRoutes config to false, then remove the per-spec workaround in options_spec.

Review asked us to avoid Rails.version stubs and rely on real Rails behavior, backed by Rails 8.1 in CI. Compare JS output to Rails to_query and add the Rails 8.1/Ruby 3.4 appraisal entry.

Review asked that the omit_undefined_query_parameters optional-path assertion live with the migration tests, not the generic Rails compatibility suite. Move that coverage and keep the migration expectations aligned with the boolean runtime config and Rails to_query serializer.

Review called the generator assertion unnecessary. The initializer template itself remains covered by generation behavior, so this only deletes the added content check.

Review asked for more migration guidance in the changelog. Expand the note to explain the warning, when to choose true, when to keep false, and that explicit null still maps to Rails nil.

Copilot

Pull request overview

Copilot reviewed 18 out of 20 changed files in this pull request and generated 2 comments.

…lib/routes.ts:269-273 When omit_undefined_query_parameters is enabled, an object inside an array can now serialize to an empty string if all of its properties are undefined; the array branch still pushes that empty sub-result, so Routes.serialize({a: [{b: undefined}, {c: 1}]}) returns &a%5B%5D%5Bc%5D=1 with a leading empty parameter. Filter empty sub-serializations in the array path the same way the object path does.

Copilot

Pull request overview

Copilot reviewed 18 out of 20 changed files in this pull request and generated 3 comments.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Reviewer asked to use Ruby 4.0 for the Rails 8.1 appraisal. Update only that CI matrix entry.

Reviewer asked to pass the transitional config into route generation and inline the single-use helper. Keep evallib default explicit while allowing specs to override it.

Reviewer asked for migration guidance in the generated initializer. Add the transitional wording without changing the configured default.

Reviewer asked to use Rails to_query expectations after omitting undefined values. Update the serializer specs, add the mixed-array case, and keep array serialization aligned with Rails empty fragments.

When include_undefined_query_parameters is false, undefined object properties are omitted before serialization, but the resulting structure should still serialize like Rails to_query. Add coverage for nested undefined array elements, nested objects that become empty inside arrays, and route-helper nested query properties.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

…omit_undefined_query_parameters # Conflicts: # CHANGELOG.md # lib/js_routes/instance.rb # lib/routes.d.ts # lib/routes.js # lib/routes.ts # lib/templates/initializer.rb # spec/js_routes/module_types/dts/routes.spec.d.ts

Remove a duplicate null/undefined compatibility assertion and clarify descriptions for the default serializer examples. This keeps the omit-undefined branch focused while leaving the Rails empty-hash array behavior documented as pending.

jbaxendale-ut · 2026-06-08T22:42:47Z

Fixed in railsware#346

jbaxendale-ut requested a review from Copilot May 27, 2026 20:37

Copilot started reviewing on behalf of jbaxendale-ut May 27, 2026 20:37 View session

jbaxendale-ut force-pushed the feature/omit_undefined_query_parameters branch from 2a00659 to caf0537 Compare May 27, 2026 20:38

Copilot AI reviewed May 27, 2026

View reviewed changes

Comment thread spec/js_routes/default_serializer_spec.rb Outdated

jbaxendale-ut mentioned this pull request May 27, 2026

undefined values treated as nil in Rails rather than omitted railsware/js-routes#345

Closed

Fix for version comparison

bc6fd25

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

jbaxendale-ut commented May 27, 2026

View reviewed changes

jbaxendale-ut added 9 commits May 28, 2026 11:41

Patch Rails 7.0 deprecator

408ef12

use proper gem version checking

d29cf34

Pass omit undefined config to JS as boolean

8403c56

Review asked that Ruby make the migration decision before generation and that the TypeScript runtime receive a non-nullable boolean. Emit true only for an explicit true setting and update generated DTS fixtures to match.

Default specs to explicit undefined behavior

2627546

Review asked us to avoid migration warnings in regular tests by specifying the new config. Default evallib and the suite-wide JsRoutes config to false, then remove the per-spec workaround in options_spec.

Use Rails serializer for nil query expectations

2fda7ef

Review asked us to avoid Rails.version stubs and rely on real Rails behavior, backed by Rails 8.1 in CI. Compare JS output to Rails to_query and add the Rails 8.1/Ruby 3.4 appraisal entry.

Remove redundant initializer template spec

ca9b798

Review called the generator assertion unnecessary. The initializer template itself remains covered by generation behavior, so this only deletes the added content check.

Explain undefined query parameter migration

ba05b28

Review asked for more migration guidance in the changelog. Expand the note to explain the warning, when to choose true, when to keep false, and that explicit null still maps to Rails nil.

jbaxendale-ut requested a review from Copilot May 28, 2026 18:15

Copilot started reviewing on behalf of jbaxendale-ut May 28, 2026 18:15 View session

Copilot AI reviewed May 28, 2026

View reviewed changes

Comment thread Appraisals Outdated

Comment thread gemfiles/rails81_sprockets_4.gemfile

jbaxendale-ut force-pushed the feature/omit_undefined_query_parameters branch from bfbc90b to 2a21149 Compare May 28, 2026 18:37

jbaxendale-ut added 2 commits May 28, 2026 14:51

Fix copilot appraisals mismatch

a2b8663

rename to include_undefined_query_parameters

3f20597

jbaxendale-ut changed the title ~~Add omit_undefined_query_parameters config flag~~ Add include_undefined_query_parameters config flag May 28, 2026

jbaxendale-ut requested a review from Copilot May 28, 2026 18:59

Copilot started reviewing on behalf of jbaxendale-ut May 28, 2026 18:59 View session

Copilot AI reviewed May 28, 2026

View reviewed changes

Comment thread lib/js_routes/instance.rb Outdated

Comment thread Readme.md

Comment thread CHANGELOG.md Outdated

jbaxendale-ut and others added 2 commits May 28, 2026 15:48

Potential fix for pull request finding

2ec9e6b

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Test Rails 8.1 on Ruby 4.0

5c1115e

Reviewer asked to use Ruby 4.0 for the Rails 8.1 appraisal. Update only that CI matrix entry.

jbaxendale-ut added 5 commits May 28, 2026 15:53

Avoid global config mutation in migration spec

b3b559f

Reviewer asked to pass the transitional config into route generation and inline the single-use helper. Keep evallib default explicit while allowing specs to override it.

Clarify undefined query initializer default

8dbd380

Reviewer asked for migration guidance in the generated initializer. Add the transitional wording without changing the configured default.

Compare undefined omission to Rails to_query

2f02d28

Reviewer asked to use Rails to_query expectations after omitting undefined values. Update the serializer specs, add the mixed-array case, and keep array serialization aligned with Rails empty fragments.

clarify config setup

4b46857

jbaxendale-ut requested a review from Copilot May 28, 2026 20:16

Copilot started reviewing on behalf of jbaxendale-ut May 28, 2026 20:16 View session

Copilot AI reviewed May 28, 2026

jbaxendale-ut force-pushed the feature/omit_undefined_query_parameters branch from 2ce9376 to c4fce45 Compare June 3, 2026 18:53

jbaxendale-ut added 9 commits June 3, 2026 15:12

Remove redundant specs from flag spec

9492d79

call missed to_query in default serializer specs

40fe3fe

flip spec behavior to be future proof

b45fc8c

Simplify evallib options initialization

49b1111

mv actual include undefined test into options_spec

abebb56

Fix CI matrix versions

d3a03fd

Mark current invalid specs as pending for the moment

5942013

Clean up default serializer specs

06cecd2

Remove a duplicate null/undefined compatibility assertion and clarify descriptions for the default serializer examples. This keeps the omit-undefined branch focused while leaving the Rails empty-hash array behavior documented as pending.

tweak wording of test case

017008d

jbaxendale-ut closed this Jun 8, 2026

jbaxendale-ut deleted the feature/omit_undefined_query_parameters branch June 8, 2026 22:42

Navigation Menu

Repository navigation

Add `include_undefined_query_parameters` config flag - #2

Add `include_undefined_query_parameters` config flag#2
jbaxendale-ut wants to merge 31 commits into
mainfrom
feature/omit_undefined_query_parameters

jbaxendale-ut commented May 27, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

jbaxendale-ut May 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

jbaxendale-ut commented Jun 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jbaxendale-ut commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

jbaxendale-ut May 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

jbaxendale-ut commented Jun 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jbaxendale-ut commented May 27, 2026 •

edited

Loading