feat(js): take a form registration back - #24

Merged
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:feat/remove-model-lifecycle
Aug 27, 2026
Merged

feat(js): take a form registration back#24
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:feat/remove-model-lifecycle

Conversation

@66Ton99

Copy link
Copy Markdown

Problem

addModel() keeps every render in forms and formInstances, and nothing takes one back out. A page that swaps rendered forms in and out of the document — a single page CRUD, a modal that loads its form — registers a model per render, so:

  • the registry grows with elements whose markup has already been removed, and getFormInstances(id) answers with them;
  • those elements keep the removed nodes alive through element.domNode and domNode.jsFormValidator;
  • initModel() calls attachDefaultEvent() unconditionally, so re-initializing the same markup stacks a second submit listener on the form and runs the whole validation twice for one submit.

Change

Three public methods, each detaching the model from the DOM nodes of the element and its children and removing the submit listener this library put on the form:

SvarohJsFormValidator.removeModel('user');// every render of one model idSvarohJsFormValidator.removeForm(document.getElementById('user'));// one rendered formSvarohJsFormValidator.removeDetachedForms();// every form no longer in the document

removeModel() answers with the number of removed registrations, removeForm() with whether it removed one, removeDetachedForms() with the number it removed. When the last registration of an id is removed, the id leaves forms as well, so the registry never answers with an element it no longer holds.

attachDefaultEvent() now keeps its listener on the node and replaces it, so initializing the same markup again validates the form once per submit instead of twice.

Nothing changes for a page that never removes a form.

Docs

New src/Resources/doc/3_24.md, linked from the README list.

Tests

Seven Jest tests in SvarohJsFormValidator.test.js covering each method, the unknown-id and unknown-node cases, that a removed form no longer runs validation on submit, and that a second initialization of the same markup does not validate twice.

npx jest — 615 tests pass (608 before). PHP checks are untouched by this change; the host PHP here is 8.3, below the ^8.4 requirement, so composer test was not run.

🤖 Generated with Claude Code

Ton Sharpand others added 2 commits August 27, 2026 22:49
A page that swaps rendered forms in and out - a single page CRUD, a modal
that loads its form - registered a model per render and had no way to
remove one. The registry kept every element, including the ones whose
markup was already gone, and "getFormInstances" answered with them.
"removeModel", "removeForm" and "removeDetachedForms" remove a
registration, detach the model from its DOM nodes and take the submit
listener off the form. Initializing the same markup again now replaces
that listener instead of stacking a second one, which used to run the
whole validation twice for one submit.
An element is attached to two nodes, not one: "createElement" attaches it to
the node the model id matched, and "initModel" moves the root element to the
form it resolved afterwards. The default rendering makes that the common case -
"form_start" writes "<form name=...>" without an id and "form_widget" puts the
id of the model on a container inside it - and "detachElement" only took the
second node back.
What that left behind:
- the container kept the removed element, and the next initialization of the
same markup read it back through "attachElement", which copies the keys of
the attached element and repoints "domNode" mid-loop, so the documented
"remove, then initialize again" flow threw a TypeError;
- "removeForm(document.getElementById(id))", the call the documentation shows,
answered with the container while the registry held the form, so it removed
nothing;
- "removeDetachedForms()" read the root node alone, so a model whose fields are
rendered outside of any form tag - "initModel" allows that - counted as
detached and was dropped while its widgets were still in the document;
- the submit listener went with the first element removed from a form, so a
second model rooted in the same form silently stopped being validated.
"detachElement" now takes both nodes through "detachNode", which keeps the
listener for as long as an element is attached to the node. "removeForm"
answers to either node. "removeDetachedForms" asks "isElementInDocument",
which looks at the whole element and its children. "attachElement" reads the
attached element once instead of re-reading it through a "domNode" it has
already overwritten.
Also: "keepFormInstances" is "replaceFormInstances", which is what it does;
the internal helpers say so in their docblocks; and an id nothing was removed
from keeps the array the registry already handed out.
Tests: ten more, on the markup Symfony actually renders. 625 pass, JavaScript
line coverage 94.96%. PHP is untouched by this change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@66Ton99

Copy link
Copy Markdown
Author

Review

Reviewed in a separate worktree, with every finding reproduced by running the code rather than by reading it. Four defects and one regression, all with the same root cause.

The seven new tests and both doc examples are built on <form id="profile">. That is not what Symfony renders: form_start() writes <form name="profile" method="post"> with no id, and form_widget() puts the id of the model on the container it writes inside the form (form_widget_compoundwidget_container_attributes). The e2e fixture of this repository, {{ form(testForm) }}, renders exactly that.

So findDomElement() matches the container, and initModel() then moves the root element to the form. The element is attached to two nodes, and detachElement() only took one of them back.

  1. The documented "remove, then initialize again" flow throws.removeModel() left jsFormValidator on the container, and the next addModel() read it back through attachElement(), which re-reads element.domNode.jsFormValidator on every iteration while the loop itself overwrites domNodeTypeError: Cannot read properties of undefined (reading 'callbacks').

  2. removeForm(document.getElementById('user')) removed nothing.getElementById() answers with the container; the registry held the form. The call in 3_24.md returned false.

  3. removeModel() left the container attached to the removed element, which is the reference the description of this PR says is taken back.

  4. removeDetachedForms() dropped live registrations. A model rendered outside of any <form> has no root node at all — initModel() allows it — and domNode && document.contains(domNode) counted it as detached while its widgets were in the document.

  5. Regression: the submit listener went with the first element removed from a form. Two models rooted in one <form> validated twice per submit before this PR and once after it, which is the improvement; after removeModel() of either one, the form was validated zero times. The listener resolves its element from the node at submit time, so it belongs to the node, not to one of its models.

Fixed in e75a67a

  • SvarohJsFormElement.widgetDomNode holds the node the model id matched, written after attachElement() so a second model in the same form cannot hand this element its container.
  • detachElement() takes both nodes back through detachNode(), which keeps the submit listener for as long as an element is attached to the node.
  • removeForm() answers to either node; removeDetachedForms() asks isElementInDocument(), which looks at the whole element and its children.
  • attachElement() reads the attached element once instead of re-reading it through a domNode it has already overwritten.
  • keepFormInstances() is replaceFormInstances(), the internal helpers say so in their docblocks, and an id nothing was removed from keeps the array the registry already handed out.
  • 3_24.md explains why getElementById() answers with the container and what removeDetachedForms() looks at.

Ten more tests, on the markup Symfony renders: attachment to both nodes, removeModel() clearing both, re-initialization of the same markup, removeForm() by the container, a model with no form tag (as a container and rendered row by row), two models in one form, plus the forms[id] repointing branch that no test reached and detachElement(null).

npx jest: 625 pass (615 before). npm run test:coverage: JavaScript line coverage 94.96%, threshold 80%, no uncovered line left in the new code. PHP is untouched by this change; the host PHP here is 8.3, below the ^8.4 requirement, so composer test was not run.

@66Ton99
66Ton99 merged commit 8679ba0 into Svaroh:mainAug 27, 2026
6 checks passed
Sign up for freeto 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.

1 participant

@66Ton99
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

feat(js): take a form registration back - #24

Merged
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:feat/remove-model-lifecycle
Aug 27, 2026
Merged

feat(js): take a form registration back#24
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:feat/remove-model-lifecycle

Conversation

@66Ton99

Copy link
Copy Markdown

Problem

addModel() keeps every render in forms and formInstances, and nothing takes one back out. A page that swaps rendered forms in and out of the document — a single page CRUD, a modal that loads its form — registers a model per render, so:

  • the registry grows with elements whose markup has already been removed, and getFormInstances(id) answers with them;
  • those elements keep the removed nodes alive through element.domNode and domNode.jsFormValidator;
  • initModel() calls attachDefaultEvent() unconditionally, so re-initializing the same markup stacks a second submit listener on the form and runs the whole validation twice for one submit.

Change

Three public methods, each detaching the model from the DOM nodes of the element and its children and removing the submit listener this library put on the form:

SvarohJsFormValidator.removeModel('user');// every render of one model idSvarohJsFormValidator.removeForm(document.getElementById('user'));// one rendered formSvarohJsFormValidator.removeDetachedForms();// every form no longer in the document

removeModel() answers with the number of removed registrations, removeForm() with whether it removed one, removeDetachedForms() with the number it removed. When the last registration of an id is removed, the id leaves forms as well, so the registry never answers with an element it no longer holds.

attachDefaultEvent() now keeps its listener on the node and replaces it, so initializing the same markup again validates the form once per submit instead of twice.

Nothing changes for a page that never removes a form.

Docs

New src/Resources/doc/3_24.md, linked from the README list.

Tests

Seven Jest tests in SvarohJsFormValidator.test.js covering each method, the unknown-id and unknown-node cases, that a removed form no longer runs validation on submit, and that a second initialization of the same markup does not validate twice.

npx jest — 615 tests pass (608 before). PHP checks are untouched by this change; the host PHP here is 8.3, below the ^8.4 requirement, so composer test was not run.

🤖 Generated with Claude Code

Ton Sharpand others added 2 commits August 27, 2026 22:49
A page that swaps rendered forms in and out - a single page CRUD, a modal
that loads its form - registered a model per render and had no way to
remove one. The registry kept every element, including the ones whose
markup was already gone, and "getFormInstances" answered with them.
"removeModel", "removeForm" and "removeDetachedForms" remove a
registration, detach the model from its DOM nodes and take the submit
listener off the form. Initializing the same markup again now replaces
that listener instead of stacking a second one, which used to run the
whole validation twice for one submit.
An element is attached to two nodes, not one: "createElement" attaches it to
the node the model id matched, and "initModel" moves the root element to the
form it resolved afterwards. The default rendering makes that the common case -
"form_start" writes "<form name=...>" without an id and "form_widget" puts the
id of the model on a container inside it - and "detachElement" only took the
second node back.
What that left behind:
- the container kept the removed element, and the next initialization of the
same markup read it back through "attachElement", which copies the keys of
the attached element and repoints "domNode" mid-loop, so the documented
"remove, then initialize again" flow threw a TypeError;
- "removeForm(document.getElementById(id))", the call the documentation shows,
answered with the container while the registry held the form, so it removed
nothing;
- "removeDetachedForms()" read the root node alone, so a model whose fields are
rendered outside of any form tag - "initModel" allows that - counted as
detached and was dropped while its widgets were still in the document;
- the submit listener went with the first element removed from a form, so a
second model rooted in the same form silently stopped being validated.
"detachElement" now takes both nodes through "detachNode", which keeps the
listener for as long as an element is attached to the node. "removeForm"
answers to either node. "removeDetachedForms" asks "isElementInDocument",
which looks at the whole element and its children. "attachElement" reads the
attached element once instead of re-reading it through a "domNode" it has
already overwritten.
Also: "keepFormInstances" is "replaceFormInstances", which is what it does;
the internal helpers say so in their docblocks; and an id nothing was removed
from keeps the array the registry already handed out.
Tests: ten more, on the markup Symfony actually renders. 625 pass, JavaScript
line coverage 94.96%. PHP is untouched by this change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@66Ton99

Copy link
Copy Markdown
Author

Review

Reviewed in a separate worktree, with every finding reproduced by running the code rather than by reading it. Four defects and one regression, all with the same root cause.

The seven new tests and both doc examples are built on <form id="profile">. That is not what Symfony renders: form_start() writes <form name="profile" method="post"> with no id, and form_widget() puts the id of the model on the container it writes inside the form (form_widget_compoundwidget_container_attributes). The e2e fixture of this repository, {{ form(testForm) }}, renders exactly that.

So findDomElement() matches the container, and initModel() then moves the root element to the form. The element is attached to two nodes, and detachElement() only took one of them back.

  1. The documented "remove, then initialize again" flow throws.removeModel() left jsFormValidator on the container, and the next addModel() read it back through attachElement(), which re-reads element.domNode.jsFormValidator on every iteration while the loop itself overwrites domNodeTypeError: Cannot read properties of undefined (reading 'callbacks').

  2. removeForm(document.getElementById('user')) removed nothing.getElementById() answers with the container; the registry held the form. The call in 3_24.md returned false.

  3. removeModel() left the container attached to the removed element, which is the reference the description of this PR says is taken back.

  4. removeDetachedForms() dropped live registrations. A model rendered outside of any <form> has no root node at all — initModel() allows it — and domNode && document.contains(domNode) counted it as detached while its widgets were in the document.

  5. Regression: the submit listener went with the first element removed from a form. Two models rooted in one <form> validated twice per submit before this PR and once after it, which is the improvement; after removeModel() of either one, the form was validated zero times. The listener resolves its element from the node at submit time, so it belongs to the node, not to one of its models.

Fixed in e75a67a

  • SvarohJsFormElement.widgetDomNode holds the node the model id matched, written after attachElement() so a second model in the same form cannot hand this element its container.
  • detachElement() takes both nodes back through detachNode(), which keeps the submit listener for as long as an element is attached to the node.
  • removeForm() answers to either node; removeDetachedForms() asks isElementInDocument(), which looks at the whole element and its children.
  • attachElement() reads the attached element once instead of re-reading it through a domNode it has already overwritten.
  • keepFormInstances() is replaceFormInstances(), the internal helpers say so in their docblocks, and an id nothing was removed from keeps the array the registry already handed out.
  • 3_24.md explains why getElementById() answers with the container and what removeDetachedForms() looks at.

Ten more tests, on the markup Symfony renders: attachment to both nodes, removeModel() clearing both, re-initialization of the same markup, removeForm() by the container, a model with no form tag (as a container and rendered row by row), two models in one form, plus the forms[id] repointing branch that no test reached and detachElement(null).

npx jest: 625 pass (615 before). npm run test:coverage: JavaScript line coverage 94.96%, threshold 80%, no uncovered line left in the new code. PHP is untouched by this change; the host PHP here is 8.3, below the ^8.4 requirement, so composer test was not run.

@66Ton99
66Ton99 merged commit 8679ba0 into Svaroh:mainAug 27, 2026
6 checks passed
Sign up for freeto 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.

1 participant

@66Ton99
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(js): take a form registration back - #24

Merged
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:feat/remove-model-lifecycle
Aug 27, 2026
Merged

feat(js): take a form registration back#24
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:feat/remove-model-lifecycle

Conversation

@66Ton99

Copy link
Copy Markdown

Problem

addModel() keeps every render in forms and formInstances, and nothing takes one back out. A page that swaps rendered forms in and out of the document — a single page CRUD, a modal that loads its form — registers a model per render, so:

  • the registry grows with elements whose markup has already been removed, and getFormInstances(id) answers with them;
  • those elements keep the removed nodes alive through element.domNode and domNode.jsFormValidator;
  • initModel() calls attachDefaultEvent() unconditionally, so re-initializing the same markup stacks a second submit listener on the form and runs the whole validation twice for one submit.

Change

Three public methods, each detaching the model from the DOM nodes of the element and its children and removing the submit listener this library put on the form:

SvarohJsFormValidator.removeModel('user');// every render of one model idSvarohJsFormValidator.removeForm(document.getElementById('user'));// one rendered formSvarohJsFormValidator.removeDetachedForms();// every form no longer in the document

removeModel() answers with the number of removed registrations, removeForm() with whether it removed one, removeDetachedForms() with the number it removed. When the last registration of an id is removed, the id leaves forms as well, so the registry never answers with an element it no longer holds.

attachDefaultEvent() now keeps its listener on the node and replaces it, so initializing the same markup again validates the form once per submit instead of twice.

Nothing changes for a page that never removes a form.

Docs

New src/Resources/doc/3_24.md, linked from the README list.

Tests

Seven Jest tests in SvarohJsFormValidator.test.js covering each method, the unknown-id and unknown-node cases, that a removed form no longer runs validation on submit, and that a second initialization of the same markup does not validate twice.

npx jest — 615 tests pass (608 before). PHP checks are untouched by this change; the host PHP here is 8.3, below the ^8.4 requirement, so composer test was not run.

🤖 Generated with Claude Code

Ton Sharpand others added 2 commits August 27, 2026 22:49
A page that swaps rendered forms in and out - a single page CRUD, a modal
that loads its form - registered a model per render and had no way to
remove one. The registry kept every element, including the ones whose
markup was already gone, and "getFormInstances" answered with them.
"removeModel", "removeForm" and "removeDetachedForms" remove a
registration, detach the model from its DOM nodes and take the submit
listener off the form. Initializing the same markup again now replaces
that listener instead of stacking a second one, which used to run the
whole validation twice for one submit.
An element is attached to two nodes, not one: "createElement" attaches it to
the node the model id matched, and "initModel" moves the root element to the
form it resolved afterwards. The default rendering makes that the common case -
"form_start" writes "<form name=...>" without an id and "form_widget" puts the
id of the model on a container inside it - and "detachElement" only took the
second node back.
What that left behind:
- the container kept the removed element, and the next initialization of the
same markup read it back through "attachElement", which copies the keys of
the attached element and repoints "domNode" mid-loop, so the documented
"remove, then initialize again" flow threw a TypeError;
- "removeForm(document.getElementById(id))", the call the documentation shows,
answered with the container while the registry held the form, so it removed
nothing;
- "removeDetachedForms()" read the root node alone, so a model whose fields are
rendered outside of any form tag - "initModel" allows that - counted as
detached and was dropped while its widgets were still in the document;
- the submit listener went with the first element removed from a form, so a
second model rooted in the same form silently stopped being validated.
"detachElement" now takes both nodes through "detachNode", which keeps the
listener for as long as an element is attached to the node. "removeForm"
answers to either node. "removeDetachedForms" asks "isElementInDocument",
which looks at the whole element and its children. "attachElement" reads the
attached element once instead of re-reading it through a "domNode" it has
already overwritten.
Also: "keepFormInstances" is "replaceFormInstances", which is what it does;
the internal helpers say so in their docblocks; and an id nothing was removed
from keeps the array the registry already handed out.
Tests: ten more, on the markup Symfony actually renders. 625 pass, JavaScript
line coverage 94.96%. PHP is untouched by this change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@66Ton99

Copy link
Copy Markdown
Author

Review

Reviewed in a separate worktree, with every finding reproduced by running the code rather than by reading it. Four defects and one regression, all with the same root cause.

The seven new tests and both doc examples are built on <form id="profile">. That is not what Symfony renders: form_start() writes <form name="profile" method="post"> with no id, and form_widget() puts the id of the model on the container it writes inside the form (form_widget_compoundwidget_container_attributes). The e2e fixture of this repository, {{ form(testForm) }}, renders exactly that.

So findDomElement() matches the container, and initModel() then moves the root element to the form. The element is attached to two nodes, and detachElement() only took one of them back.

  1. The documented "remove, then initialize again" flow throws.removeModel() left jsFormValidator on the container, and the next addModel() read it back through attachElement(), which re-reads element.domNode.jsFormValidator on every iteration while the loop itself overwrites domNodeTypeError: Cannot read properties of undefined (reading 'callbacks').

  2. removeForm(document.getElementById('user')) removed nothing.getElementById() answers with the container; the registry held the form. The call in 3_24.md returned false.

  3. removeModel() left the container attached to the removed element, which is the reference the description of this PR says is taken back.

  4. removeDetachedForms() dropped live registrations. A model rendered outside of any <form> has no root node at all — initModel() allows it — and domNode && document.contains(domNode) counted it as detached while its widgets were in the document.

  5. Regression: the submit listener went with the first element removed from a form. Two models rooted in one <form> validated twice per submit before this PR and once after it, which is the improvement; after removeModel() of either one, the form was validated zero times. The listener resolves its element from the node at submit time, so it belongs to the node, not to one of its models.

Fixed in e75a67a

  • SvarohJsFormElement.widgetDomNode holds the node the model id matched, written after attachElement() so a second model in the same form cannot hand this element its container.
  • detachElement() takes both nodes back through detachNode(), which keeps the submit listener for as long as an element is attached to the node.
  • removeForm() answers to either node; removeDetachedForms() asks isElementInDocument(), which looks at the whole element and its children.
  • attachElement() reads the attached element once instead of re-reading it through a domNode it has already overwritten.
  • keepFormInstances() is replaceFormInstances(), the internal helpers say so in their docblocks, and an id nothing was removed from keeps the array the registry already handed out.
  • 3_24.md explains why getElementById() answers with the container and what removeDetachedForms() looks at.

Ten more tests, on the markup Symfony renders: attachment to both nodes, removeModel() clearing both, re-initialization of the same markup, removeForm() by the container, a model with no form tag (as a container and rendered row by row), two models in one form, plus the forms[id] repointing branch that no test reached and detachElement(null).

npx jest: 625 pass (615 before). npm run test:coverage: JavaScript line coverage 94.96%, threshold 80%, no uncovered line left in the new code. PHP is untouched by this change; the host PHP here is 8.3, below the ^8.4 requirement, so composer test was not run.

@66Ton99
66Ton99 merged commit 8679ba0 into Svaroh:mainAug 27, 2026
6 checks passed
Sign up for freeto 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.

1 participant

@66Ton99
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(js): take a form registration back - #24

Merged
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:feat/remove-model-lifecycle
Aug 27, 2026
Merged

feat(js): take a form registration back#24
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:feat/remove-model-lifecycle

Conversation

@66Ton99

Copy link
Copy Markdown

Problem

addModel() keeps every render in forms and formInstances, and nothing takes one back out. A page that swaps rendered forms in and out of the document — a single page CRUD, a modal that loads its form — registers a model per render, so:

  • the registry grows with elements whose markup has already been removed, and getFormInstances(id) answers with them;
  • those elements keep the removed nodes alive through element.domNode and domNode.jsFormValidator;
  • initModel() calls attachDefaultEvent() unconditionally, so re-initializing the same markup stacks a second submit listener on the form and runs the whole validation twice for one submit.

Change

Three public methods, each detaching the model from the DOM nodes of the element and its children and removing the submit listener this library put on the form:

SvarohJsFormValidator.removeModel('user');// every render of one model idSvarohJsFormValidator.removeForm(document.getElementById('user'));// one rendered formSvarohJsFormValidator.removeDetachedForms();// every form no longer in the document

removeModel() answers with the number of removed registrations, removeForm() with whether it removed one, removeDetachedForms() with the number it removed. When the last registration of an id is removed, the id leaves forms as well, so the registry never answers with an element it no longer holds.

attachDefaultEvent() now keeps its listener on the node and replaces it, so initializing the same markup again validates the form once per submit instead of twice.

Nothing changes for a page that never removes a form.

Docs

New src/Resources/doc/3_24.md, linked from the README list.

Tests

Seven Jest tests in SvarohJsFormValidator.test.js covering each method, the unknown-id and unknown-node cases, that a removed form no longer runs validation on submit, and that a second initialization of the same markup does not validate twice.

npx jest — 615 tests pass (608 before). PHP checks are untouched by this change; the host PHP here is 8.3, below the ^8.4 requirement, so composer test was not run.

🤖 Generated with Claude Code

Ton Sharpand others added 2 commits August 27, 2026 22:49
A page that swaps rendered forms in and out - a single page CRUD, a modal
that loads its form - registered a model per render and had no way to
remove one. The registry kept every element, including the ones whose
markup was already gone, and "getFormInstances" answered with them.
"removeModel", "removeForm" and "removeDetachedForms" remove a
registration, detach the model from its DOM nodes and take the submit
listener off the form. Initializing the same markup again now replaces
that listener instead of stacking a second one, which used to run the
whole validation twice for one submit.
An element is attached to two nodes, not one: "createElement" attaches it to
the node the model id matched, and "initModel" moves the root element to the
form it resolved afterwards. The default rendering makes that the common case -
"form_start" writes "<form name=...>" without an id and "form_widget" puts the
id of the model on a container inside it - and "detachElement" only took the
second node back.
What that left behind:
- the container kept the removed element, and the next initialization of the
same markup read it back through "attachElement", which copies the keys of
the attached element and repoints "domNode" mid-loop, so the documented
"remove, then initialize again" flow threw a TypeError;
- "removeForm(document.getElementById(id))", the call the documentation shows,
answered with the container while the registry held the form, so it removed
nothing;
- "removeDetachedForms()" read the root node alone, so a model whose fields are
rendered outside of any form tag - "initModel" allows that - counted as
detached and was dropped while its widgets were still in the document;
- the submit listener went with the first element removed from a form, so a
second model rooted in the same form silently stopped being validated.
"detachElement" now takes both nodes through "detachNode", which keeps the
listener for as long as an element is attached to the node. "removeForm"
answers to either node. "removeDetachedForms" asks "isElementInDocument",
which looks at the whole element and its children. "attachElement" reads the
attached element once instead of re-reading it through a "domNode" it has
already overwritten.
Also: "keepFormInstances" is "replaceFormInstances", which is what it does;
the internal helpers say so in their docblocks; and an id nothing was removed
from keeps the array the registry already handed out.
Tests: ten more, on the markup Symfony actually renders. 625 pass, JavaScript
line coverage 94.96%. PHP is untouched by this change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@66Ton99

Copy link
Copy Markdown
Author

Review

Reviewed in a separate worktree, with every finding reproduced by running the code rather than by reading it. Four defects and one regression, all with the same root cause.

The seven new tests and both doc examples are built on <form id="profile">. That is not what Symfony renders: form_start() writes <form name="profile" method="post"> with no id, and form_widget() puts the id of the model on the container it writes inside the form (form_widget_compoundwidget_container_attributes). The e2e fixture of this repository, {{ form(testForm) }}, renders exactly that.

So findDomElement() matches the container, and initModel() then moves the root element to the form. The element is attached to two nodes, and detachElement() only took one of them back.

  1. The documented "remove, then initialize again" flow throws.removeModel() left jsFormValidator on the container, and the next addModel() read it back through attachElement(), which re-reads element.domNode.jsFormValidator on every iteration while the loop itself overwrites domNodeTypeError: Cannot read properties of undefined (reading 'callbacks').

  2. removeForm(document.getElementById('user')) removed nothing.getElementById() answers with the container; the registry held the form. The call in 3_24.md returned false.

  3. removeModel() left the container attached to the removed element, which is the reference the description of this PR says is taken back.

  4. removeDetachedForms() dropped live registrations. A model rendered outside of any <form> has no root node at all — initModel() allows it — and domNode && document.contains(domNode) counted it as detached while its widgets were in the document.

  5. Regression: the submit listener went with the first element removed from a form. Two models rooted in one <form> validated twice per submit before this PR and once after it, which is the improvement; after removeModel() of either one, the form was validated zero times. The listener resolves its element from the node at submit time, so it belongs to the node, not to one of its models.

Fixed in e75a67a

  • SvarohJsFormElement.widgetDomNode holds the node the model id matched, written after attachElement() so a second model in the same form cannot hand this element its container.
  • detachElement() takes both nodes back through detachNode(), which keeps the submit listener for as long as an element is attached to the node.
  • removeForm() answers to either node; removeDetachedForms() asks isElementInDocument(), which looks at the whole element and its children.
  • attachElement() reads the attached element once instead of re-reading it through a domNode it has already overwritten.
  • keepFormInstances() is replaceFormInstances(), the internal helpers say so in their docblocks, and an id nothing was removed from keeps the array the registry already handed out.
  • 3_24.md explains why getElementById() answers with the container and what removeDetachedForms() looks at.

Ten more tests, on the markup Symfony renders: attachment to both nodes, removeModel() clearing both, re-initialization of the same markup, removeForm() by the container, a model with no form tag (as a container and rendered row by row), two models in one form, plus the forms[id] repointing branch that no test reached and detachElement(null).

npx jest: 625 pass (615 before). npm run test:coverage: JavaScript line coverage 94.96%, threshold 80%, no uncovered line left in the new code. PHP is untouched by this change; the host PHP here is 8.3, below the ^8.4 requirement, so composer test was not run.

@66Ton99
66Ton99 merged commit 8679ba0 into Svaroh:mainAug 27, 2026
6 checks passed
Sign up for freeto 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.

1 participant

@66Ton99
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

feat(js): take a form registration back - #24

Merged
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:feat/remove-model-lifecycle
Aug 27, 2026
Merged

feat(js): take a form registration back#24
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:feat/remove-model-lifecycle

Conversation

@66Ton99

Copy link
Copy Markdown

Problem

addModel() keeps every render in forms and formInstances, and nothing takes one back out. A page that swaps rendered forms in and out of the document — a single page CRUD, a modal that loads its form — registers a model per render, so:

  • the registry grows with elements whose markup has already been removed, and getFormInstances(id) answers with them;
  • those elements keep the removed nodes alive through element.domNode and domNode.jsFormValidator;
  • initModel() calls attachDefaultEvent() unconditionally, so re-initializing the same markup stacks a second submit listener on the form and runs the whole validation twice for one submit.

Change

Three public methods, each detaching the model from the DOM nodes of the element and its children and removing the submit listener this library put on the form:

SvarohJsFormValidator.removeModel('user');// every render of one model idSvarohJsFormValidator.removeForm(document.getElementById('user'));// one rendered formSvarohJsFormValidator.removeDetachedForms();// every form no longer in the document

removeModel() answers with the number of removed registrations, removeForm() with whether it removed one, removeDetachedForms() with the number it removed. When the last registration of an id is removed, the id leaves forms as well, so the registry never answers with an element it no longer holds.

attachDefaultEvent() now keeps its listener on the node and replaces it, so initializing the same markup again validates the form once per submit instead of twice.

Nothing changes for a page that never removes a form.

Docs

New src/Resources/doc/3_24.md, linked from the README list.

Tests

Seven Jest tests in SvarohJsFormValidator.test.js covering each method, the unknown-id and unknown-node cases, that a removed form no longer runs validation on submit, and that a second initialization of the same markup does not validate twice.

npx jest — 615 tests pass (608 before). PHP checks are untouched by this change; the host PHP here is 8.3, below the ^8.4 requirement, so composer test was not run.

🤖 Generated with Claude Code

Ton Sharpand others added 2 commits August 27, 2026 22:49
A page that swaps rendered forms in and out - a single page CRUD, a modal
that loads its form - registered a model per render and had no way to
remove one. The registry kept every element, including the ones whose
markup was already gone, and "getFormInstances" answered with them.
"removeModel", "removeForm" and "removeDetachedForms" remove a
registration, detach the model from its DOM nodes and take the submit
listener off the form. Initializing the same markup again now replaces
that listener instead of stacking a second one, which used to run the
whole validation twice for one submit.
An element is attached to two nodes, not one: "createElement" attaches it to
the node the model id matched, and "initModel" moves the root element to the
form it resolved afterwards. The default rendering makes that the common case -
"form_start" writes "<form name=...>" without an id and "form_widget" puts the
id of the model on a container inside it - and "detachElement" only took the
second node back.
What that left behind:
- the container kept the removed element, and the next initialization of the
same markup read it back through "attachElement", which copies the keys of
the attached element and repoints "domNode" mid-loop, so the documented
"remove, then initialize again" flow threw a TypeError;
- "removeForm(document.getElementById(id))", the call the documentation shows,
answered with the container while the registry held the form, so it removed
nothing;
- "removeDetachedForms()" read the root node alone, so a model whose fields are
rendered outside of any form tag - "initModel" allows that - counted as
detached and was dropped while its widgets were still in the document;
- the submit listener went with the first element removed from a form, so a
second model rooted in the same form silently stopped being validated.
"detachElement" now takes both nodes through "detachNode", which keeps the
listener for as long as an element is attached to the node. "removeForm"
answers to either node. "removeDetachedForms" asks "isElementInDocument",
which looks at the whole element and its children. "attachElement" reads the
attached element once instead of re-reading it through a "domNode" it has
already overwritten.
Also: "keepFormInstances" is "replaceFormInstances", which is what it does;
the internal helpers say so in their docblocks; and an id nothing was removed
from keeps the array the registry already handed out.
Tests: ten more, on the markup Symfony actually renders. 625 pass, JavaScript
line coverage 94.96%. PHP is untouched by this change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@66Ton99

Copy link
Copy Markdown
Author

Review

Reviewed in a separate worktree, with every finding reproduced by running the code rather than by reading it. Four defects and one regression, all with the same root cause.

The seven new tests and both doc examples are built on <form id="profile">. That is not what Symfony renders: form_start() writes <form name="profile" method="post"> with no id, and form_widget() puts the id of the model on the container it writes inside the form (form_widget_compoundwidget_container_attributes). The e2e fixture of this repository, {{ form(testForm) }}, renders exactly that.

So findDomElement() matches the container, and initModel() then moves the root element to the form. The element is attached to two nodes, and detachElement() only took one of them back.

  1. The documented "remove, then initialize again" flow throws.removeModel() left jsFormValidator on the container, and the next addModel() read it back through attachElement(), which re-reads element.domNode.jsFormValidator on every iteration while the loop itself overwrites domNodeTypeError: Cannot read properties of undefined (reading 'callbacks').

  2. removeForm(document.getElementById('user')) removed nothing.getElementById() answers with the container; the registry held the form. The call in 3_24.md returned false.

  3. removeModel() left the container attached to the removed element, which is the reference the description of this PR says is taken back.

  4. removeDetachedForms() dropped live registrations. A model rendered outside of any <form> has no root node at all — initModel() allows it — and domNode && document.contains(domNode) counted it as detached while its widgets were in the document.

  5. Regression: the submit listener went with the first element removed from a form. Two models rooted in one <form> validated twice per submit before this PR and once after it, which is the improvement; after removeModel() of either one, the form was validated zero times. The listener resolves its element from the node at submit time, so it belongs to the node, not to one of its models.

Fixed in e75a67a

  • SvarohJsFormElement.widgetDomNode holds the node the model id matched, written after attachElement() so a second model in the same form cannot hand this element its container.
  • detachElement() takes both nodes back through detachNode(), which keeps the submit listener for as long as an element is attached to the node.
  • removeForm() answers to either node; removeDetachedForms() asks isElementInDocument(), which looks at the whole element and its children.
  • attachElement() reads the attached element once instead of re-reading it through a domNode it has already overwritten.
  • keepFormInstances() is replaceFormInstances(), the internal helpers say so in their docblocks, and an id nothing was removed from keeps the array the registry already handed out.
  • 3_24.md explains why getElementById() answers with the container and what removeDetachedForms() looks at.

Ten more tests, on the markup Symfony renders: attachment to both nodes, removeModel() clearing both, re-initialization of the same markup, removeForm() by the container, a model with no form tag (as a container and rendered row by row), two models in one form, plus the forms[id] repointing branch that no test reached and detachElement(null).

npx jest: 625 pass (615 before). npm run test:coverage: JavaScript line coverage 94.96%, threshold 80%, no uncovered line left in the new code. PHP is untouched by this change; the host PHP here is 8.3, below the ^8.4 requirement, so composer test was not run.

@66Ton99
66Ton99 merged commit 8679ba0 into Svaroh:mainAug 27, 2026
6 checks passed
Sign up for freeto 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.

1 participant

@66Ton99
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(js): take a form registration back - #24

Merged
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:feat/remove-model-lifecycle
Aug 27, 2026
Merged

feat(js): take a form registration back#24
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:feat/remove-model-lifecycle

Conversation

@66Ton99

Copy link
Copy Markdown

Problem

addModel() keeps every render in forms and formInstances, and nothing takes one back out. A page that swaps rendered forms in and out of the document — a single page CRUD, a modal that loads its form — registers a model per render, so:

  • the registry grows with elements whose markup has already been removed, and getFormInstances(id) answers with them;
  • those elements keep the removed nodes alive through element.domNode and domNode.jsFormValidator;
  • initModel() calls attachDefaultEvent() unconditionally, so re-initializing the same markup stacks a second submit listener on the form and runs the whole validation twice for one submit.

Change

Three public methods, each detaching the model from the DOM nodes of the element and its children and removing the submit listener this library put on the form:

SvarohJsFormValidator.removeModel('user');// every render of one model idSvarohJsFormValidator.removeForm(document.getElementById('user'));// one rendered formSvarohJsFormValidator.removeDetachedForms();// every form no longer in the document

removeModel() answers with the number of removed registrations, removeForm() with whether it removed one, removeDetachedForms() with the number it removed. When the last registration of an id is removed, the id leaves forms as well, so the registry never answers with an element it no longer holds.

attachDefaultEvent() now keeps its listener on the node and replaces it, so initializing the same markup again validates the form once per submit instead of twice.

Nothing changes for a page that never removes a form.

Docs

New src/Resources/doc/3_24.md, linked from the README list.

Tests

Seven Jest tests in SvarohJsFormValidator.test.js covering each method, the unknown-id and unknown-node cases, that a removed form no longer runs validation on submit, and that a second initialization of the same markup does not validate twice.

npx jest — 615 tests pass (608 before). PHP checks are untouched by this change; the host PHP here is 8.3, below the ^8.4 requirement, so composer test was not run.

🤖 Generated with Claude Code

Ton Sharpand others added 2 commits August 27, 2026 22:49
A page that swaps rendered forms in and out - a single page CRUD, a modal
that loads its form - registered a model per render and had no way to
remove one. The registry kept every element, including the ones whose
markup was already gone, and "getFormInstances" answered with them.
"removeModel", "removeForm" and "removeDetachedForms" remove a
registration, detach the model from its DOM nodes and take the submit
listener off the form. Initializing the same markup again now replaces
that listener instead of stacking a second one, which used to run the
whole validation twice for one submit.
An element is attached to two nodes, not one: "createElement" attaches it to
the node the model id matched, and "initModel" moves the root element to the
form it resolved afterwards. The default rendering makes that the common case -
"form_start" writes "<form name=...>" without an id and "form_widget" puts the
id of the model on a container inside it - and "detachElement" only took the
second node back.
What that left behind:
- the container kept the removed element, and the next initialization of the
same markup read it back through "attachElement", which copies the keys of
the attached element and repoints "domNode" mid-loop, so the documented
"remove, then initialize again" flow threw a TypeError;
- "removeForm(document.getElementById(id))", the call the documentation shows,
answered with the container while the registry held the form, so it removed
nothing;
- "removeDetachedForms()" read the root node alone, so a model whose fields are
rendered outside of any form tag - "initModel" allows that - counted as
detached and was dropped while its widgets were still in the document;
- the submit listener went with the first element removed from a form, so a
second model rooted in the same form silently stopped being validated.
"detachElement" now takes both nodes through "detachNode", which keeps the
listener for as long as an element is attached to the node. "removeForm"
answers to either node. "removeDetachedForms" asks "isElementInDocument",
which looks at the whole element and its children. "attachElement" reads the
attached element once instead of re-reading it through a "domNode" it has
already overwritten.
Also: "keepFormInstances" is "replaceFormInstances", which is what it does;
the internal helpers say so in their docblocks; and an id nothing was removed
from keeps the array the registry already handed out.
Tests: ten more, on the markup Symfony actually renders. 625 pass, JavaScript
line coverage 94.96%. PHP is untouched by this change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@66Ton99

Copy link
Copy Markdown
Author

Review

Reviewed in a separate worktree, with every finding reproduced by running the code rather than by reading it. Four defects and one regression, all with the same root cause.

The seven new tests and both doc examples are built on <form id="profile">. That is not what Symfony renders: form_start() writes <form name="profile" method="post"> with no id, and form_widget() puts the id of the model on the container it writes inside the form (form_widget_compoundwidget_container_attributes). The e2e fixture of this repository, {{ form(testForm) }}, renders exactly that.

So findDomElement() matches the container, and initModel() then moves the root element to the form. The element is attached to two nodes, and detachElement() only took one of them back.

  1. The documented "remove, then initialize again" flow throws.removeModel() left jsFormValidator on the container, and the next addModel() read it back through attachElement(), which re-reads element.domNode.jsFormValidator on every iteration while the loop itself overwrites domNodeTypeError: Cannot read properties of undefined (reading 'callbacks').

  2. removeForm(document.getElementById('user')) removed nothing.getElementById() answers with the container; the registry held the form. The call in 3_24.md returned false.

  3. removeModel() left the container attached to the removed element, which is the reference the description of this PR says is taken back.

  4. removeDetachedForms() dropped live registrations. A model rendered outside of any <form> has no root node at all — initModel() allows it — and domNode && document.contains(domNode) counted it as detached while its widgets were in the document.

  5. Regression: the submit listener went with the first element removed from a form. Two models rooted in one <form> validated twice per submit before this PR and once after it, which is the improvement; after removeModel() of either one, the form was validated zero times. The listener resolves its element from the node at submit time, so it belongs to the node, not to one of its models.

Fixed in e75a67a

  • SvarohJsFormElement.widgetDomNode holds the node the model id matched, written after attachElement() so a second model in the same form cannot hand this element its container.
  • detachElement() takes both nodes back through detachNode(), which keeps the submit listener for as long as an element is attached to the node.
  • removeForm() answers to either node; removeDetachedForms() asks isElementInDocument(), which looks at the whole element and its children.
  • attachElement() reads the attached element once instead of re-reading it through a domNode it has already overwritten.
  • keepFormInstances() is replaceFormInstances(), the internal helpers say so in their docblocks, and an id nothing was removed from keeps the array the registry already handed out.
  • 3_24.md explains why getElementById() answers with the container and what removeDetachedForms() looks at.

Ten more tests, on the markup Symfony renders: attachment to both nodes, removeModel() clearing both, re-initialization of the same markup, removeForm() by the container, a model with no form tag (as a container and rendered row by row), two models in one form, plus the forms[id] repointing branch that no test reached and detachElement(null).

npx jest: 625 pass (615 before). npm run test:coverage: JavaScript line coverage 94.96%, threshold 80%, no uncovered line left in the new code. PHP is untouched by this change; the host PHP here is 8.3, below the ^8.4 requirement, so composer test was not run.

@66Ton99
66Ton99 merged commit 8679ba0 into Svaroh:mainAug 27, 2026
6 checks passed
Sign up for freeto 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.

1 participant

@66Ton99
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(js): take a form registration back - #24

Merged
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:feat/remove-model-lifecycle
Aug 27, 2026
Merged

feat(js): take a form registration back#24
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:feat/remove-model-lifecycle

Conversation

@66Ton99

Copy link
Copy Markdown

Problem

addModel() keeps every render in forms and formInstances, and nothing takes one back out. A page that swaps rendered forms in and out of the document — a single page CRUD, a modal that loads its form — registers a model per render, so:

  • the registry grows with elements whose markup has already been removed, and getFormInstances(id) answers with them;
  • those elements keep the removed nodes alive through element.domNode and domNode.jsFormValidator;
  • initModel() calls attachDefaultEvent() unconditionally, so re-initializing the same markup stacks a second submit listener on the form and runs the whole validation twice for one submit.

Change

Three public methods, each detaching the model from the DOM nodes of the element and its children and removing the submit listener this library put on the form:

SvarohJsFormValidator.removeModel('user');// every render of one model idSvarohJsFormValidator.removeForm(document.getElementById('user'));// one rendered formSvarohJsFormValidator.removeDetachedForms();// every form no longer in the document

removeModel() answers with the number of removed registrations, removeForm() with whether it removed one, removeDetachedForms() with the number it removed. When the last registration of an id is removed, the id leaves forms as well, so the registry never answers with an element it no longer holds.

attachDefaultEvent() now keeps its listener on the node and replaces it, so initializing the same markup again validates the form once per submit instead of twice.

Nothing changes for a page that never removes a form.

Docs

New src/Resources/doc/3_24.md, linked from the README list.

Tests

Seven Jest tests in SvarohJsFormValidator.test.js covering each method, the unknown-id and unknown-node cases, that a removed form no longer runs validation on submit, and that a second initialization of the same markup does not validate twice.

npx jest — 615 tests pass (608 before). PHP checks are untouched by this change; the host PHP here is 8.3, below the ^8.4 requirement, so composer test was not run.

🤖 Generated with Claude Code

Ton Sharpand others added 2 commits August 27, 2026 22:49
A page that swaps rendered forms in and out - a single page CRUD, a modal
that loads its form - registered a model per render and had no way to
remove one. The registry kept every element, including the ones whose
markup was already gone, and "getFormInstances" answered with them.
"removeModel", "removeForm" and "removeDetachedForms" remove a
registration, detach the model from its DOM nodes and take the submit
listener off the form. Initializing the same markup again now replaces
that listener instead of stacking a second one, which used to run the
whole validation twice for one submit.
An element is attached to two nodes, not one: "createElement" attaches it to
the node the model id matched, and "initModel" moves the root element to the
form it resolved afterwards. The default rendering makes that the common case -
"form_start" writes "<form name=...>" without an id and "form_widget" puts the
id of the model on a container inside it - and "detachElement" only took the
second node back.
What that left behind:
- the container kept the removed element, and the next initialization of the
same markup read it back through "attachElement", which copies the keys of
the attached element and repoints "domNode" mid-loop, so the documented
"remove, then initialize again" flow threw a TypeError;
- "removeForm(document.getElementById(id))", the call the documentation shows,
answered with the container while the registry held the form, so it removed
nothing;
- "removeDetachedForms()" read the root node alone, so a model whose fields are
rendered outside of any form tag - "initModel" allows that - counted as
detached and was dropped while its widgets were still in the document;
- the submit listener went with the first element removed from a form, so a
second model rooted in the same form silently stopped being validated.
"detachElement" now takes both nodes through "detachNode", which keeps the
listener for as long as an element is attached to the node. "removeForm"
answers to either node. "removeDetachedForms" asks "isElementInDocument",
which looks at the whole element and its children. "attachElement" reads the
attached element once instead of re-reading it through a "domNode" it has
already overwritten.
Also: "keepFormInstances" is "replaceFormInstances", which is what it does;
the internal helpers say so in their docblocks; and an id nothing was removed
from keeps the array the registry already handed out.
Tests: ten more, on the markup Symfony actually renders. 625 pass, JavaScript
line coverage 94.96%. PHP is untouched by this change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@66Ton99

Copy link
Copy Markdown
Author

Review

Reviewed in a separate worktree, with every finding reproduced by running the code rather than by reading it. Four defects and one regression, all with the same root cause.

The seven new tests and both doc examples are built on <form id="profile">. That is not what Symfony renders: form_start() writes <form name="profile" method="post"> with no id, and form_widget() puts the id of the model on the container it writes inside the form (form_widget_compoundwidget_container_attributes). The e2e fixture of this repository, {{ form(testForm) }}, renders exactly that.

So findDomElement() matches the container, and initModel() then moves the root element to the form. The element is attached to two nodes, and detachElement() only took one of them back.

  1. The documented "remove, then initialize again" flow throws.removeModel() left jsFormValidator on the container, and the next addModel() read it back through attachElement(), which re-reads element.domNode.jsFormValidator on every iteration while the loop itself overwrites domNodeTypeError: Cannot read properties of undefined (reading 'callbacks').

  2. removeForm(document.getElementById('user')) removed nothing.getElementById() answers with the container; the registry held the form. The call in 3_24.md returned false.

  3. removeModel() left the container attached to the removed element, which is the reference the description of this PR says is taken back.

  4. removeDetachedForms() dropped live registrations. A model rendered outside of any <form> has no root node at all — initModel() allows it — and domNode && document.contains(domNode) counted it as detached while its widgets were in the document.

  5. Regression: the submit listener went with the first element removed from a form. Two models rooted in one <form> validated twice per submit before this PR and once after it, which is the improvement; after removeModel() of either one, the form was validated zero times. The listener resolves its element from the node at submit time, so it belongs to the node, not to one of its models.

Fixed in e75a67a

  • SvarohJsFormElement.widgetDomNode holds the node the model id matched, written after attachElement() so a second model in the same form cannot hand this element its container.
  • detachElement() takes both nodes back through detachNode(), which keeps the submit listener for as long as an element is attached to the node.
  • removeForm() answers to either node; removeDetachedForms() asks isElementInDocument(), which looks at the whole element and its children.
  • attachElement() reads the attached element once instead of re-reading it through a domNode it has already overwritten.
  • keepFormInstances() is replaceFormInstances(), the internal helpers say so in their docblocks, and an id nothing was removed from keeps the array the registry already handed out.
  • 3_24.md explains why getElementById() answers with the container and what removeDetachedForms() looks at.

Ten more tests, on the markup Symfony renders: attachment to both nodes, removeModel() clearing both, re-initialization of the same markup, removeForm() by the container, a model with no form tag (as a container and rendered row by row), two models in one form, plus the forms[id] repointing branch that no test reached and detachElement(null).

npx jest: 625 pass (615 before). npm run test:coverage: JavaScript line coverage 94.96%, threshold 80%, no uncovered line left in the new code. PHP is untouched by this change; the host PHP here is 8.3, below the ^8.4 requirement, so composer test was not run.

@66Ton99
66Ton99 merged commit 8679ba0 into Svaroh:mainAug 27, 2026
6 checks passed
Sign up for freeto 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.

1 participant

@66Ton99
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

feat(js): take a form registration back - #24

Merged
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:feat/remove-model-lifecycle
Aug 27, 2026
Merged

feat(js): take a form registration back#24
66Ton99 merged 2 commits into
Svaroh:mainfrom
Web20:feat/remove-model-lifecycle

Conversation

@66Ton99

Copy link
Copy Markdown

Problem

addModel() keeps every render in forms and formInstances, and nothing takes one back out. A page that swaps rendered forms in and out of the document — a single page CRUD, a modal that loads its form — registers a model per render, so:

  • the registry grows with elements whose markup has already been removed, and getFormInstances(id) answers with them;
  • those elements keep the removed nodes alive through element.domNode and domNode.jsFormValidator;
  • initModel() calls attachDefaultEvent() unconditionally, so re-initializing the same markup stacks a second submit listener on the form and runs the whole validation twice for one submit.

Change

Three public methods, each detaching the model from the DOM nodes of the element and its children and removing the submit listener this library put on the form:

SvarohJsFormValidator.removeModel('user');// every render of one model idSvarohJsFormValidator.removeForm(document.getElementById('user'));// one rendered formSvarohJsFormValidator.removeDetachedForms();// every form no longer in the document

removeModel() answers with the number of removed registrations, removeForm() with whether it removed one, removeDetachedForms() with the number it removed. When the last registration of an id is removed, the id leaves forms as well, so the registry never answers with an element it no longer holds.

attachDefaultEvent() now keeps its listener on the node and replaces it, so initializing the same markup again validates the form once per submit instead of twice.

Nothing changes for a page that never removes a form.

Docs

New src/Resources/doc/3_24.md, linked from the README list.

Tests

Seven Jest tests in SvarohJsFormValidator.test.js covering each method, the unknown-id and unknown-node cases, that a removed form no longer runs validation on submit, and that a second initialization of the same markup does not validate twice.

npx jest — 615 tests pass (608 before). PHP checks are untouched by this change; the host PHP here is 8.3, below the ^8.4 requirement, so composer test was not run.

🤖 Generated with Claude Code

Ton Sharpand others added 2 commits August 27, 2026 22:49
A page that swaps rendered forms in and out - a single page CRUD, a modal
that loads its form - registered a model per render and had no way to
remove one. The registry kept every element, including the ones whose
markup was already gone, and "getFormInstances" answered with them.
"removeModel", "removeForm" and "removeDetachedForms" remove a
registration, detach the model from its DOM nodes and take the submit
listener off the form. Initializing the same markup again now replaces
that listener instead of stacking a second one, which used to run the
whole validation twice for one submit.
An element is attached to two nodes, not one: "createElement" attaches it to
the node the model id matched, and "initModel" moves the root element to the
form it resolved afterwards. The default rendering makes that the common case -
"form_start" writes "<form name=...>" without an id and "form_widget" puts the
id of the model on a container inside it - and "detachElement" only took the
second node back.
What that left behind:
- the container kept the removed element, and the next initialization of the
same markup read it back through "attachElement", which copies the keys of
the attached element and repoints "domNode" mid-loop, so the documented
"remove, then initialize again" flow threw a TypeError;
- "removeForm(document.getElementById(id))", the call the documentation shows,
answered with the container while the registry held the form, so it removed
nothing;
- "removeDetachedForms()" read the root node alone, so a model whose fields are
rendered outside of any form tag - "initModel" allows that - counted as
detached and was dropped while its widgets were still in the document;
- the submit listener went with the first element removed from a form, so a
second model rooted in the same form silently stopped being validated.
"detachElement" now takes both nodes through "detachNode", which keeps the
listener for as long as an element is attached to the node. "removeForm"
answers to either node. "removeDetachedForms" asks "isElementInDocument",
which looks at the whole element and its children. "attachElement" reads the
attached element once instead of re-reading it through a "domNode" it has
already overwritten.
Also: "keepFormInstances" is "replaceFormInstances", which is what it does;
the internal helpers say so in their docblocks; and an id nothing was removed
from keeps the array the registry already handed out.
Tests: ten more, on the markup Symfony actually renders. 625 pass, JavaScript
line coverage 94.96%. PHP is untouched by this change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@66Ton99

Copy link
Copy Markdown
Author

Review

Reviewed in a separate worktree, with every finding reproduced by running the code rather than by reading it. Four defects and one regression, all with the same root cause.

The seven new tests and both doc examples are built on <form id="profile">. That is not what Symfony renders: form_start() writes <form name="profile" method="post"> with no id, and form_widget() puts the id of the model on the container it writes inside the form (form_widget_compoundwidget_container_attributes). The e2e fixture of this repository, {{ form(testForm) }}, renders exactly that.

So findDomElement() matches the container, and initModel() then moves the root element to the form. The element is attached to two nodes, and detachElement() only took one of them back.

  1. The documented "remove, then initialize again" flow throws.removeModel() left jsFormValidator on the container, and the next addModel() read it back through attachElement(), which re-reads element.domNode.jsFormValidator on every iteration while the loop itself overwrites domNodeTypeError: Cannot read properties of undefined (reading 'callbacks').

  2. removeForm(document.getElementById('user')) removed nothing.getElementById() answers with the container; the registry held the form. The call in 3_24.md returned false.

  3. removeModel() left the container attached to the removed element, which is the reference the description of this PR says is taken back.

  4. removeDetachedForms() dropped live registrations. A model rendered outside of any <form> has no root node at all — initModel() allows it — and domNode && document.contains(domNode) counted it as detached while its widgets were in the document.

  5. Regression: the submit listener went with the first element removed from a form. Two models rooted in one <form> validated twice per submit before this PR and once after it, which is the improvement; after removeModel() of either one, the form was validated zero times. The listener resolves its element from the node at submit time, so it belongs to the node, not to one of its models.

Fixed in e75a67a

  • SvarohJsFormElement.widgetDomNode holds the node the model id matched, written after attachElement() so a second model in the same form cannot hand this element its container.
  • detachElement() takes both nodes back through detachNode(), which keeps the submit listener for as long as an element is attached to the node.
  • removeForm() answers to either node; removeDetachedForms() asks isElementInDocument(), which looks at the whole element and its children.
  • attachElement() reads the attached element once instead of re-reading it through a domNode it has already overwritten.
  • keepFormInstances() is replaceFormInstances(), the internal helpers say so in their docblocks, and an id nothing was removed from keeps the array the registry already handed out.
  • 3_24.md explains why getElementById() answers with the container and what removeDetachedForms() looks at.

Ten more tests, on the markup Symfony renders: attachment to both nodes, removeModel() clearing both, re-initialization of the same markup, removeForm() by the container, a model with no form tag (as a container and rendered row by row), two models in one form, plus the forms[id] repointing branch that no test reached and detachElement(null).

npx jest: 625 pass (615 before). npm run test:coverage: JavaScript line coverage 94.96%, threshold 80%, no uncovered line left in the new code. PHP is untouched by this change; the host PHP here is 8.3, below the ^8.4 requirement, so composer test was not run.

@66Ton99
66Ton99 merged commit 8679ba0 into Svaroh:mainAug 27, 2026
6 checks passed
Sign up for freeto 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.

1 participant

@66Ton99