Skip to content

Post-merge-review: Fix template-require-form-method: throw on bad config; default enabled#2698

Merged
NullVoxPopuli merged 5 commits intoember-cli:masterfrom
johanrd:day_fix/template-require-form-method
Apr 15, 2026
Merged

Post-merge-review: Fix template-require-form-method: throw on bad config; default enabled#2698
NullVoxPopuli merged 5 commits intoember-cli:masterfrom
johanrd:day_fix/template-require-form-method

Conversation

@johanrd
Copy link
Copy Markdown
Contributor

@johanrd johanrd commented Apr 13, 2026

Summary

  • Rule is enabled by default (aligns with ember-template-lint upstream) — bare <form> and invalid method values are flagged without any configuration
  • Pass false to explicitly disable: 'template-require-form-method': ['error', false]
  • parseConfig throws on invalid allowedMethods values instead of silently disabling
  • Schema accepts boolean root config for parity with upstream

Test plan

  • No options → rule enabled, bare <form> flagged
  • No options → <form method="NOT_A_VALID_METHOD"> flagged
  • options: [true] → rule enabled with POST,GET,DIALOG
  • options: [false] → rule explicitly disabled
  • options: [{ allowedMethods: ['PATCH'] }] → throws configuration error

@johanrd johanrd marked this pull request as ready for review April 13, 2026 17:04
@johanrd johanrd changed the title Fix template-require-form-method: throw on bad config; default disabled Post-merge-review: Fix template-require-form-method: throw on bad config; default disabled Apr 13, 2026
}

return false;
// Invalid configuration (e.g. unknown method in allowedMethods). Throw a
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comments describing 'what' instead of 'why' are usually worthless. Bad bot

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

thanks, fixed now

'<div method="randomType"></div>',
// Default behavior: when no options are provided, the rule is disabled.
// A bare <form> should NOT error.
'<form></form>',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is supposed to be invalid, according to ember-template-lint

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

thanks, fixed now

johanrd added 4 commits April 15, 2026 08:15
parseConfig now throws on invalid allowedMethods (matches upstream's
strictness instead of silently disabling). parseConfig(undefined)
returns false so the rule doesn't run when options are omitted
(matches upstream default). Schema accepts boolean root for parity.
…ber-template-lint

Align with upstream: bare <form> and invalid method values are now flagged
without explicit configuration. Pass `false` to opt out.
@johanrd johanrd force-pushed the day_fix/template-require-form-method branch from 3d2e871 to 34225ac Compare April 15, 2026 06:16
@johanrd johanrd changed the title Post-merge-review: Fix template-require-form-method: throw on bad config; default disabled Post-merge-review: Fix template-require-form-method: throw on bad config; default enabled Apr 15, 2026
options: [{ allowedMethods: ['get'] }],
code: '<form method="GET"></form>',
},
'<form method="POST"></form>',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why'd all these get removed? do we not want to test what happens with no options anymore?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

thanks, reverted, now, was only moved to wrappers on the sort { options: [true], code: ', but that was unnecessary for most.

…h no options

  With the rule default-enabled, `options: [true]` is redundant with no
  options. Switch most valid cases to bare strings so the tests
  exercise the default user path. Keep one `options: [true]` and one
  `options: [false]` as sentinels documenting that the explicit boolean
  forms still work.
@NullVoxPopuli NullVoxPopuli merged commit 6f846ab into ember-cli:master Apr 15, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants