Open
Conversation
* refactor: remove all lodash dependencies - Remove lodash.has, lodash.isfunction, lodash.isempty, lodash.isobject - Replace has() with Object.prototype.hasOwnProperty.call() - Replace isEmpty() with Object.keys().length === 0 - Replace isObject() with typeof === 'object' && !== null - Replace isFunction() with typeof === 'function' - Remove Npm.depends block entirely - No behavioral changes * refactor(templating-runtime): remove lodash.has dependency - Replace has() with Object.prototype.hasOwnProperty.call() - Remove Npm.depends block - No behavioral changes * refactor(caching-html-compiler): remove lodash.isempty dependency - Replace isEmpty() with Object.keys().length > 0 - Remove Npm.depends block - No behavioral changes * refactor(templating-tools): remove lodash.isempty dependency - Replace isEmpty() with Object.keys().length > 0 - Remove Npm.depends block - No behavioral changes * chore: remove .npm/package directories for removed lodash deps These directories were generated by Meteor for the lodash Npm.depends that no longer exist. The spacebars-compiler .npm directory is kept as it still depends on uglify-js. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(spacebars-compiler): remove uglify-js dependency uglify-js was only used for pretty-printing compiled template code (compress: false, mangle: false), not actual minification. The beautify function now returns code as-is. Updated test expectations accordingly. This removes the last external npm dependency from Blaze packages. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(blaze): extract hasOwn and isObject into utils.js module Add shared utility helpers imported via ES modules instead of repeating Object.prototype.hasOwnProperty.call inline across multiple files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(blaze): use isObject helper in template.js Replace remaining inline typeof/null checks with the shared isObject utility from utils.js. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: replace uglify-js with native syntax validation in spacebars-compiler --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…478) * ♻️ (blaze/attrs): optimize attribute updates by caching last values to avoid redundant DOM updates and ensure handlers are cleaned up properly * ♻️ (blaze/attrs.js): add simple cache to getUrlProtocol for performance * ✅ (benchmarks): add HTML benchmarks for ElementAttributesUpdater and getUrlProtocol Add standalone HTML files to benchmark and compare performance of ElementAttributesUpdater (_lastValues cache) and getUrlProtocol (with and without protocol cache). These benchmarks help quantify the performance impact of recent optimizations and provide reproducible evidence for future improvements. * Add detailed documentation to Blaze attribute handlers Expanded and clarified documentation for all attribute handler classes in packages/blaze/attrs.js. Added JSDoc comments, usage examples, and security notes for each handler type, improving maintainability and developer understanding. No functional changes were made. * fix: revert test-app to stable Meteor, address PR review feedback - Revert test-app Meteor config to METEOR@3.0-rc.2 (master) to fix CI build failure caused by babel-compiler beta bug - Fix LRU -> FIFO in cache eviction comments (matches actual behavior) - Improve shouldUpdate logic to short-circuit on strict equality first - Fix benchmark comparison to match actual attrs.js implementation - Remove unnecessary comment in benchmark file * fix: simplify shouldUpdate, fix BooleanHandler docs, add _clearProtocolCache - Simplify shouldUpdate to strict equality + null check only, since materializer always normalizes values to string|null (drop String()) - Fix BooleanHandler docstring to accurately describe that handlers receive normalized text values, not raw booleans - Add Blaze._clearProtocolCache() for testing and cache reset support - Update benchmark to match simplified shouldUpdate logic --------- Co-authored-by: Jan Küster <jkuester@uni-bremen.de>
* refactor(htmljs): convert ES5 syntax to ES2015+ - var → const/let - Template literals for error messages and string interpolation - Arrow functions (IDENTITY, isSVGElement, etc.) - .startsWith() instead of .slice()/.substring() comparisons - Spread syntax instead of .apply() - No behavioral changes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(blaze-tools): convert ES5 syntax to ES2015+ - var → const/let - Template literals for string building - Arrow functions (hasToJS, unicodeClass, etc.) - .includes() instead of .indexOf() !== -1 - String.padStart() instead of manual padding - No behavioral changes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(html-tools): convert ES5 syntax to ES2015+ - var → const/let - Arrow functions for callbacks and short functions - Template literals for error messages - .includes() instead of .indexOf() !== -1 - Spread syntax instead of .apply() - No behavioral changes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(observe-sequence): convert ES5 syntax to ES2015+ - var → const/let (preserving intentional globals) - Arrow functions for callbacks - Template literals for warn messages - Default parameters - Spread syntax for console.warn arguments - Fix for-loop scoping (let i declared before loop when used after) - No behavioral changes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(spacebars-compiler): convert ES5 syntax to ES2015+ - var → const/let - Arrow functions (makeObjectLiteral, constant, etc.) - Template literals for code generation strings - .includes() instead of .indexOf() !== -1 - self=this → arrow functions in codegen.js - No behavioral changes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(templating-tools): convert ES5 syntax to ES2015+ - var → const/let - Template literals for error messages - No behavioral changes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(blaze): convert ES5 syntax to ES2015+ - var → const/let - self=this → arrow functions (view.js, attrs.js, lookup.js, template.js) - HelperMap constructor+prototype → ES6 class (internal only) - TeardownCallback constructor+prototype → ES6 class (internal only) - Arrow functions for short callbacks and IIFEs - Template literals for error/warn messages - .startsWith() instead of .substring() comparisons - .includes() instead of .indexOf() !== -1 - Spread syntax instead of .apply() - Preserves .extend() pattern for AttributeHandler and Visitor - Preserves constructor functions with instanceof guards (View, Template, DOMRange) - No behavioral changes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(spacebars): convert ES5 syntax to ES2015+ - var → const/let - Spread syntax instead of .apply(null, args) (8 instances) - Arrow functions (tripleEquals, etc.) - No behavioral changes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(templating-runtime): convert ES5 syntax to ES2015+ - var → const/let - Arrow functions for forEach callbacks - Template literals for error messages - .includes() instead of .indexOf() !== -1 - No behavioral changes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(spacebars-tests): convert ES5 syntax to ES2015+ - var → const/let (~1150 conversions) - Fix duplicate declarations in same scope (let + reassignment) - Fix const without initializer → let - Fix for-loop scoping where var was used after loop body - No behavioral changes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: resolve merge conflict regressions from release-3.1.0 - attrs.js: replace undefined `self` with `this` in UrlHandler (3 occurrences) - templating.js: add missing semicolon on migrate reassignment - dynamic.js: restore var→const conversion Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: convert missed function expressions to arrow functions Address review comments from jankapunkt on PR #488: - tojs.js: IIFE and inner callback → arrow functions - charref.js: getCodePoints and isLegalCodepoint → arrow functions Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Jan Küster <jkuester@uni-bremen.de>
* docs: prepare improvement of arch overview * Initial plan * docs: add comprehensive architecture overview with package roles and build/runtime stages Co-authored-by: jankapunkt <1135285+jankapunkt@users.noreply.github.com> * docs: add Key Terms section explaining Spacebars, Template, Template Instance, and View Co-authored-by: jankapunkt <1135285+jankapunkt@users.noreply.github.com> * docs: fix relationship diagram to accurately show Template Instance references View Co-authored-by: jankapunkt <1135285+jankapunkt@users.noreply.github.com> * docs: remove redundant info --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Nacho Codoñer <igcogi@gmail.com>
* docs: fix badges and links * docs: fix link in README for Blaze Tests badge * docs: Fix typo in README about Blaze package
* feat: dual-mode jQuery/native DOM backend Detect jQuery at load time. If present, all existing jQuery code paths are used (zero breaking changes). If absent, native DOM APIs are used. A deprecation message is logged when jQuery is detected. Ref #490 * fix: address Copilot review findings in native DOM backend - Fix bindEventCapturer bug: check elem.contains(matched) instead of elem.contains(target) to prevent firing when selector matches an ancestor outside the container - Add text node guard (nodeType check) in delegateEvents and bindEventCapturer to prevent TypeError on text node targets - Deduplicate getContext() — branch only on jQuery legacy support check - Fix WeakMap comment: value is Array<{wrapper, eventType}>, not {wrapper, type} - Fix handler key comment: eventType stored in entry, not used as key --------- Co-authored-by: Jan Küster <jkuester@uni-bremen.de>
📝 WalkthroughWalkthroughThis PR modernizes the Blaze templating system by removing all Lodash dependencies across multiple packages, refactoring code to use modern JavaScript constructs (const/let, arrow functions, template literals, includes()), making the DOM backend jQuery-optional with native fallbacks, adding JavaScript URL security controls with caching, and updating documentation. The changes span approximately 40 files. Changes
Possibly related issues
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/templating-tools/html-scanner-tests.js (1)
12-18:⚠️ Potential issue | 🟠 MajorFix
instanceofguard precedence incheckError.Line 16 is currently parsed as
(!e) instanceof TemplatingTools.CompileError, so the condition always evaluates to false and unexpected exceptions are silently swallowed instead of being rethrown.Proposed fix
- if (! e instanceof TemplatingTools.CompileError) { + if (!(e instanceof TemplatingTools.CompileError)) { throw e; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/templating-tools/html-scanner-tests.js` around lines 12 - 18, The instanceof guard in the checkError function is using incorrect precedence: change the condition from "! e instanceof TemplatingTools.CompileError" to test the instance correctly (e.g. "!(e instanceof TemplatingTools.CompileError)" or "if (e instanceof TemplatingTools.CompileError) { ... } else { throw e }") so that unexpected exceptions thrown by f() are rethrown; update the checkError function and its catch block to use the corrected instanceof check referencing TemplatingTools.CompileError and the caught variable e.packages/htmljs/visitors.js (1)
191-200:⚠️ Potential issue | 🟠 MajorFilter inherited properties when iterating attributes.
The
for...inloops at lines 191 and 304 iterate attribute dictionaries without filtering inherited enumerable properties. This allows properties from polluted or custom prototypes to flow throughvisitAttributeand serialize as HTML attributes. The inconsistency with_assign(which guards via_hasOwnProperty.call()at lines 21–27) leaves these paths unprotected.Use
Object.keys()instead offor...in:
- Line 191:
for (const k of Object.keys(oldAttrs))- Line 304:
for (const k of Object.keys(attrs))Also apply the same fix to
flattenAttributesinhtml.js(around line 214) for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/htmljs/visitors.js` around lines 191 - 200, The for...in loops iterate attribute maps and allow inherited enumerable properties to be processed; change them to iterate only own keys using Object.keys to prevent prototype-polluted attributes from reaching visitAttribute and being serialized: in visitors.js replace the loop over oldAttrs (where oldValue is read and visitAttribute is called and _assign is used) with a for (const k of Object.keys(oldAttrs)) loop, and similarly replace the loop over attrs (the second occurrence that calls visitAttribute) with for (const k of Object.keys(attrs)); also make the analogous change in flattenAttributes (html.js, function flattenAttributes) to iterate Object.keys(attrs) instead of for...in so behavior matches the _assign/_hasOwnProperty protections already used elsewhere.packages/html-tools/charref.js (1)
2247-2259:⚠️ Potential issue | 🟠 MajorUse prototype-safe maps for the entity matcher cache.
Lines 2250 and 2255 iterate plain-object lookup tables with
for...in. If an enumerable property is added toObject.prototypebefore module initialization, it will be treated as an entity key and poison the generated matchers. Build these tables with null prototypes and iterate own keys only.Proposed fix
-const getNamedEntityByFirstChar = {}; +const getNamedEntityByFirstChar = Object.create(null); (function () { - const namedEntitiesByFirstChar = {}; - for (const ent in ENTITIES) { + const namedEntitiesByFirstChar = Object.create(null); + for (const ent of Object.keys(ENTITIES)) { const chr = ent.charAt(1); namedEntitiesByFirstChar[chr] = (namedEntitiesByFirstChar[chr] || []); namedEntitiesByFirstChar[chr].push(ent.slice(2)); } - for (const chr in namedEntitiesByFirstChar) { + for (const chr of Object.keys(namedEntitiesByFirstChar)) { getNamedEntityByFirstChar[chr] = makeRegexMatcher( new RegExp('^&' + chr + '(?:' + namedEntitiesByFirstChar[chr].join('|') + ')')); } })();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/html-tools/charref.js` around lines 2247 - 2259, The cache objects should be prototype-safe: create getNamedEntityByFirstChar and namedEntitiesByFirstChar with null prototypes (use Object.create(null)) and iterate only own keys (use Object.keys or for-of over Object.keys) instead of for...in; specifically, build namedEntitiesByFirstChar with Object.create(null), loop over Object.keys(ENTITIES) when collecting entries, then loop over Object.keys(namedEntitiesByFirstChar) when creating matchers for getNamedEntityByFirstChar; keep references to ENTITIES and makeRegexMatcher and assign into getNamedEntityByFirstChar as before.packages/spacebars-compiler/codegen.js (1)
302-328:⚠️ Potential issue | 🔴 CriticalPotential null dereference on
tagArgs.The comment on line 306 states "tagArgs may be null", but line 307 calls
tagArgs.forEach(...)directly without a null check. This will throw aTypeErroriftagArgsisnullorundefined.🐛 Proposed fix to add null guard
// tagArgs may be null - tagArgs.forEach((arg) => { + (tagArgs || []).forEach((arg) => { const argCode = this.codeGenArgValue(arg);Alternatively, use optional chaining with nullish coalescing or an early return pattern.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/spacebars-compiler/codegen.js` around lines 302 - 328, In codeGenMustacheArgs, tagArgs may be null but the function calls tagArgs.forEach directly; add a null guard (e.g., early return when tagArgs is null/undefined or iterate only when tagArgs exists) so the function doesn't throw a TypeError—update codeGenMustacheArgs to check tagArgs before calling forEach and preserve existing behavior (produce null/empty args/kwArgs and still append Spacebars.kw when needed).
🧹 Nitpick comments (9)
README.md (1)
39-40: Consider hyphenating the compound adjective.The phrase "Blaze related news" would be more grammatically correct as "Blaze-related news" when the compound adjective precedes the noun.
📝 Proposed grammar improvement
-definitely [join us on the Meteor forums](http://forums.meteor.com). +definitely [join us on the Meteor forums](https://forums.meteor.com).Also consider this optional grammar fix for line 39:
-Also, to stay up-to-date on all Blaze related news and the community you should +Also, to stay up-to-date on all Blaze-related news and the community you shouldNote: Also updated the forum link to use HTTPS for security.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 39 - 40, Change the phrase "Blaze related news" to the compound adjective "Blaze-related news" in the README and update the forum URL in the same sentence from "http://forums.meteor.com" to "https://forums.meteor.com"; locate the sentence containing the link text "[join us on the Meteor forums]" and replace the text and URL accordingly to maintain correct grammar and a secure HTTPS link.packages/observe-sequence/observe_sequence.js (6)
406-436: Implicit global:seqChangedToCursorlacks declaration keyword.Same issue - this function should be declared with
const.♻️ Suggested fix
-seqChangedToCursor = function (lastSeqArray, cursor, callbacks) { +const seqChangedToCursor = function (lastSeqArray, cursor, callbacks) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/observe-sequence/observe_sequence.js` around lines 406 - 436, The function seqChangedToCursor is assigned without a declaration, creating an implicit global; change its definition to use a proper declaration (e.g., add "const" before seqChangedToCursor) so it becomes a local constant function, leaving the internal logic (seqArray, observeHandle, callbacks) unchanged; ensure no other references rely on a global and run lint/tests to confirm no scope-related regressions.
270-273: Variable shadowing in callback: intentional but slightly obscure.The destructured
idin[id, pos]shadows the outeridparameter fromaddedBefore. While this works correctly (the iteration updates positions for all entries, not just the added one), consider renaming toentryIdorkeyfor clarity.✨ Optional: Rename to avoid shadowing
- Object.entries(posCur).forEach(([id, pos]) => { + Object.entries(posCur).forEach(([key, pos]) => { if (pos >= position) - posCur[id]++; + posCur[key]++; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/observe-sequence/observe_sequence.js` around lines 270 - 273, In the addedBefore function where Object.entries(posCur).forEach(([id, pos]) => { ... }) is used, rename the destructured variable `id` to something non-shadowing (e.g., `entryId` or `key`) and update the body to use that new name (e.g., posCur[entryId]++) so it no longer shadows the outer `id` parameter; this keeps the logic unchanged but improves clarity around the outer `id` and the callback identifier.
373-404: Implicit global:seqChangedToArraylacks declaration keyword.Same issue as
seqChangedToEmpty- this function assignment creates an implicit global.♻️ Suggested fix
-seqChangedToArray = function (lastSeqArray, array, callbacks) { +const seqChangedToArray = function (lastSeqArray, array, callbacks) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/observe-sequence/observe_sequence.js` around lines 373 - 404, Declare seqChangedToArray to avoid creating an implicit global by adding a proper declaration (e.g., use "const seqChangedToArray = function(...)" or a "function seqChangedToArray(...)" form) instead of the current bare assignment; update the definition that produces seqArray (references: seqChangedToArray, idsUsed, idStringify, Random.id, warn) so the function is locally declared and does not leak to the global scope.
331-336: Same shadowing pattern.The
[id, pos]destructuring shadows the outeridparameter fromremoved.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/observe-sequence/observe_sequence.js` around lines 331 - 336, The loop destructures ([id, pos]) which shadows the outer removed parameter named id; rename the loop key to avoid shadowing (e.g., ([entryId, pos])) and update the body to reference posCur[entryId] and compare pos against prevPosition (computed from posCur[idStringify(id)]), so the outer id/removed stays intact; references: removed parameter name id, posCur, idStringify, and the forEach callback where ([id, pos]) appears.
369-371: Implicit global:seqChangedToEmptyis not declared withconst/let.While this line wasn't modified in this PR (no
~marker), the function is assigned without a declaration keyword, creating an implicit global. This is inconsistent with the modernization applied elsewhere in this file.♻️ Suggested fix
-seqChangedToEmpty = function (lastSeqArray, callbacks) { +const seqChangedToEmpty = function (lastSeqArray, callbacks) { return []; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/observe-sequence/observe_sequence.js` around lines 369 - 371, The function seqChangedToEmpty is currently assigned without a declaration, creating an implicit global; update its definition to use a local declaration (e.g., prepend const or let) so it is properly scoped (replace the bare assignment to seqChangedToEmpty with a declared binding for the function named seqChangedToEmpty used alongside other locals in this module).
313-318: Same shadowing pattern as above.The
[id, elCurPosition]destructuring shadows the outeridparameter frommovedBefore.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/observe-sequence/observe_sequence.js` around lines 313 - 318, The inner callback is shadowing the outer parameter id from movedBefore; rename the destructured key in the forEach from [id, elCurPosition] to a different identifier (e.g., [entryId, elCurPosition]) and update all uses inside the callback (posCur[entryId]-- / posCur[entryId]++). This preserves the outer movedBefore id parameter and avoids shadowing in the Object.entries(...).forEach callback.packages/blaze-tools/tojs.js (1)
90-106: Redundant array initialization.Line 95 re-initializes
attrsArray = []when it was already set to[]on line 93 (two lines before, inside the sameif (attrs)block).♻️ Remove redundant initialization
attrsArray = []; if (HTML.isArray(attrs)) { - attrsArray = []; for (let i = 0; i < attrs.length; i++) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/blaze-tools/tojs.js` around lines 90 - 106, The code redundantly sets attrsArray = [] twice inside the if (attrs) block; remove the second initialization so attrsArray is only initialized once when attrs is truthy. Edit the block surrounding attrsArray, attrs, HTML.isArray, hasToJS and generateAttrsDictionary to delete the duplicate attrsArray = [] (the one immediately after HTML.isArray(attrs) check) and leave the rest of the loop logic unchanged.packages/html-tools/parse.js (1)
48-65: Consider usingletfor consistency.Line 51 uses
var endTagwhile the rest of the file has been modernized to useconst/let. Whilevarworks here due to hoisting across the try/catch block, usingletwith a declaration before the try block would be more consistent with the modernization effort.♻️ Proposed consistency fix
const posBefore = scanner.pos; + let endTag; try { - var endTag = getHTMLToken(scanner); + endTag = getHTMLToken(scanner); } catch (e) { // ignore errors from getTemplateTag }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/html-tools/parse.js` around lines 48 - 65, Replace the function-scoped "var endTag" with a block-scoped declaration: declare "let endTag" immediately before the try block and assign to it inside the try by calling getHTMLToken(scanner); keep the existing catch behavior and subsequent use of endTag (the if checking endTag.t/isEnd and scanner.fatal) unchanged so behavior is identical but uses modern let scoping instead of var.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@OVERVIEW.md`:
- Around line 199-206: The unlabeled fenced code blocks flagged by MD040 should
each include a language identifier (use "text")—specifically add ```text before
the block that begins ".html file → templating-compiler …", the block that
begins "Template function called …", and the block that begins "Template
(Blueprint) …" so all three fenced blocks are labeled and markdownlint MD040 is
satisfied.
- Line 18: Replace the branch-pinned raw URL image link in the markdown (the
current
"https://raw.githubusercontent.com/meteor/blaze/architecture/images/architecture_packages.svg"
reference) with a repository-relative path to the asset (e.g. use the relative
Markdown image URL pointing at architecture/images/architecture_packages.svg) so
the docs render reliably across branches and forks; update the image markdown in
OVERVIEW.md accordingly.
In `@packages/blaze/attrs.js`:
- Around line 746-762: The current short-circuit uses shouldUpdate to skip
calling handler.update when the new value equals lastValues[k], which prevents
handlers that reconcile against the live DOM (e.g., for
value/checked/class/style) from restoring template-owned state after
user/external mutation; change the logic in the block that references
lastValues, handlers, handler and shouldUpdate so that handler.update(elem,
oldValue, value) is always called regardless of shouldUpdate, but only update
handler.value and lastValues[k] (and perform the value === null cleanup that
deletes handlers[k] and lastValues[k]) when shouldUpdate is true — keep the same
calls and symbols (handler.update, handler.value, lastValues, handlers, value,
oldValue, elem) and only move the update invocation out of the short-circuit
guard.
In `@packages/blaze/dombackend.js`:
- Around line 94-109: The undelegateEvents implementation currently removes all
wrappers for a given handler regardless of the passed type; update
undelegateEvents to only remove entries whose entry.eventType equals the
provided type: look up handlerMap via _delegateMap.get(elem), get entries =
handlerMap.get(handler), iterate and only call
elem.removeEventListener(entry.eventType, entry.wrapper) for entries where
entry.eventType === type (or matches the provided type string), remove those
matching entries from the entries collection, and only call
handlerMap.delete(handler) if no entries remain for that handler; reference
undelegateEvents, _delegateMap, handlerMap, entries, and
entry.eventType/entry.wrapper to locate the change.
- Around line 36-42: DOMBackend.parseHTML's native branch currently returns
template.content.childNodes which keeps <script> elements, whereas the jQuery
branch ($jq.parseHTML(...)) excludes scripts by default; update the native path
in DOMBackend.parseHTML to filter out SCRIPT nodes before returning so both
branches behave consistently. In practice, after creating template and
populating template.innerHTML, iterate over template.content.childNodes (or use
querySelectorAll) and return only nodes whose nodeName !== 'SCRIPT' (or nodeType
!== Node.ELEMENT_NODE || tagName !== 'SCRIPT') so script elements are excluded
from the returned array.
In `@packages/html-tools/scanner.js`:
- Around line 34-37: The truncation logic for pastInput is wrong because
substring(-CONTEXT_AMOUNT) treats negative indices as 0; in the block that sets
pastInput (using variables input, pos, CONTEXT_AMOUNT and the pastInput
variable), replace the use of substring(-CONTEXT_AMOUNT) with
slice(-CONTEXT_AMOUNT) so pastInput = `...${pastInput.slice(-CONTEXT_AMOUNT)}`
to correctly return the trailing N characters when pastInput.length >
CONTEXT_AMOUNT.
In `@packages/spacebars-compiler/compile_tests.js`:
- Around line 25-30: The postProcess function currently uses the regex
/\/\/.*$/mg which naively strips from // to end-of-line and can remove //
sequences inside string literals (e.g., "http://..."); replace that regex usage
in postProcess with a safe line-by-line comment stripper that scans each line
character-by-character, tracking quote state and escapes so it only removes //
when not inside single or double quotes (implement a helper like
stripLineCommentsPreservingStrings used from postProcess), then continue with
the existing whitespace collapse and paren-trimming steps; ensure the new code
references postProcess and removes the /\/\/.*$/mg usage.
In `@packages/spacebars-compiler/templatetag.js`:
- Around line 359-366: TemplateTag.parseCompleteTag is creating a local variable
scanner but then calls TemplateTag.parse(scannerOrString), which can create a
different Scanner and desynchronize cursor positions; change the call to use the
local scanner variable (i.e., call TemplateTag.parse(scanner)) so parsing,
parseFragment, validation and error position reporting all use the same Scanner
instance (update any other places in this function that pass scannerOrString
into internal parse functions to use scanner).
In `@packages/templating-runtime/templating.js`:
- Around line 161-164: The function signature for Template._migrateTemplate
currently declares an unused third parameter `migrate`; remove it and compute
migration intent locally instead so caller input isn't expected. Change the
signature to Template._migrateTemplate = function (templateName, newTemplate) {
and replace the reassignment line with a local const, e.g. const migrate =
Template.__pendingReplacement.includes(templateName); (or delete any unused
`migrate` variable entirely if it’s not referenced later).
In `@README.md`:
- Around line 5-6: The badge image tag lacks alternative text; update the
README.md badge <img> element (inside the <a
href="https://github.com/meteor/blaze/actions/workflows/blaze-tests.yml"> link)
to include a descriptive alt attribute (e.g., alt="GitHub Actions — blaze-tests
workflow status") so screen readers can convey the badge meaning; ensure the alt
text is concise and specific to the workflow name/status.
---
Outside diff comments:
In `@packages/html-tools/charref.js`:
- Around line 2247-2259: The cache objects should be prototype-safe: create
getNamedEntityByFirstChar and namedEntitiesByFirstChar with null prototypes (use
Object.create(null)) and iterate only own keys (use Object.keys or for-of over
Object.keys) instead of for...in; specifically, build namedEntitiesByFirstChar
with Object.create(null), loop over Object.keys(ENTITIES) when collecting
entries, then loop over Object.keys(namedEntitiesByFirstChar) when creating
matchers for getNamedEntityByFirstChar; keep references to ENTITIES and
makeRegexMatcher and assign into getNamedEntityByFirstChar as before.
In `@packages/htmljs/visitors.js`:
- Around line 191-200: The for...in loops iterate attribute maps and allow
inherited enumerable properties to be processed; change them to iterate only own
keys using Object.keys to prevent prototype-polluted attributes from reaching
visitAttribute and being serialized: in visitors.js replace the loop over
oldAttrs (where oldValue is read and visitAttribute is called and _assign is
used) with a for (const k of Object.keys(oldAttrs)) loop, and similarly replace
the loop over attrs (the second occurrence that calls visitAttribute) with for
(const k of Object.keys(attrs)); also make the analogous change in
flattenAttributes (html.js, function flattenAttributes) to iterate
Object.keys(attrs) instead of for...in so behavior matches the
_assign/_hasOwnProperty protections already used elsewhere.
In `@packages/spacebars-compiler/codegen.js`:
- Around line 302-328: In codeGenMustacheArgs, tagArgs may be null but the
function calls tagArgs.forEach directly; add a null guard (e.g., early return
when tagArgs is null/undefined or iterate only when tagArgs exists) so the
function doesn't throw a TypeError—update codeGenMustacheArgs to check tagArgs
before calling forEach and preserve existing behavior (produce null/empty
args/kwArgs and still append Spacebars.kw when needed).
In `@packages/templating-tools/html-scanner-tests.js`:
- Around line 12-18: The instanceof guard in the checkError function is using
incorrect precedence: change the condition from "! e instanceof
TemplatingTools.CompileError" to test the instance correctly (e.g. "!(e
instanceof TemplatingTools.CompileError)" or "if (e instanceof
TemplatingTools.CompileError) { ... } else { throw e }") so that unexpected
exceptions thrown by f() are rethrown; update the checkError function and its
catch block to use the corrected instanceof check referencing
TemplatingTools.CompileError and the caught variable e.
---
Nitpick comments:
In `@packages/blaze-tools/tojs.js`:
- Around line 90-106: The code redundantly sets attrsArray = [] twice inside the
if (attrs) block; remove the second initialization so attrsArray is only
initialized once when attrs is truthy. Edit the block surrounding attrsArray,
attrs, HTML.isArray, hasToJS and generateAttrsDictionary to delete the duplicate
attrsArray = [] (the one immediately after HTML.isArray(attrs) check) and leave
the rest of the loop logic unchanged.
In `@packages/html-tools/parse.js`:
- Around line 48-65: Replace the function-scoped "var endTag" with a
block-scoped declaration: declare "let endTag" immediately before the try block
and assign to it inside the try by calling getHTMLToken(scanner); keep the
existing catch behavior and subsequent use of endTag (the if checking
endTag.t/isEnd and scanner.fatal) unchanged so behavior is identical but uses
modern let scoping instead of var.
In `@packages/observe-sequence/observe_sequence.js`:
- Around line 406-436: The function seqChangedToCursor is assigned without a
declaration, creating an implicit global; change its definition to use a proper
declaration (e.g., add "const" before seqChangedToCursor) so it becomes a local
constant function, leaving the internal logic (seqArray, observeHandle,
callbacks) unchanged; ensure no other references rely on a global and run
lint/tests to confirm no scope-related regressions.
- Around line 270-273: In the addedBefore function where
Object.entries(posCur).forEach(([id, pos]) => { ... }) is used, rename the
destructured variable `id` to something non-shadowing (e.g., `entryId` or `key`)
and update the body to use that new name (e.g., posCur[entryId]++) so it no
longer shadows the outer `id` parameter; this keeps the logic unchanged but
improves clarity around the outer `id` and the callback identifier.
- Around line 373-404: Declare seqChangedToArray to avoid creating an implicit
global by adding a proper declaration (e.g., use "const seqChangedToArray =
function(...)" or a "function seqChangedToArray(...)" form) instead of the
current bare assignment; update the definition that produces seqArray
(references: seqChangedToArray, idsUsed, idStringify, Random.id, warn) so the
function is locally declared and does not leak to the global scope.
- Around line 331-336: The loop destructures ([id, pos]) which shadows the outer
removed parameter named id; rename the loop key to avoid shadowing (e.g.,
([entryId, pos])) and update the body to reference posCur[entryId] and compare
pos against prevPosition (computed from posCur[idStringify(id)]), so the outer
id/removed stays intact; references: removed parameter name id, posCur,
idStringify, and the forEach callback where ([id, pos]) appears.
- Around line 369-371: The function seqChangedToEmpty is currently assigned
without a declaration, creating an implicit global; update its definition to use
a local declaration (e.g., prepend const or let) so it is properly scoped
(replace the bare assignment to seqChangedToEmpty with a declared binding for
the function named seqChangedToEmpty used alongside other locals in this
module).
- Around line 313-318: The inner callback is shadowing the outer parameter id
from movedBefore; rename the destructured key in the forEach from [id,
elCurPosition] to a different identifier (e.g., [entryId, elCurPosition]) and
update all uses inside the callback (posCur[entryId]-- / posCur[entryId]++).
This preserves the outer movedBefore id parameter and avoids shadowing in the
Object.entries(...).forEach callback.
In `@README.md`:
- Around line 39-40: Change the phrase "Blaze related news" to the compound
adjective "Blaze-related news" in the README and update the forum URL in the
same sentence from "http://forums.meteor.com" to "https://forums.meteor.com";
locate the sentence containing the link text "[join us on the Meteor forums]"
and replace the text and URL accordingly to maintain correct grammar and a
secure HTTPS link.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a4eaaee7-9299-4d96-ba0c-d4770b95bbaf
⛔ Files ignored due to path filters (7)
images/architecture-static-html.pngis excluded by!**/*.pngimages/architecture-templating-tools.pngis excluded by!**/*.pngimages/architecture/architecture-static-html.svgis excluded by!**/*.svgimages/architecture/architecture-templating-tools.svgis excluded by!**/*.svgimages/architecture/architecture_packages.svgis excluded by!**/*.svgimages/architecture_packages.pngis excluded by!**/*.pngimages/architecture_packages.svgis excluded by!**/*.svg
📒 Files selected for processing (77)
OVERVIEW.mdREADME.mdimages/architecture/architecture-static-html.graphmlimages/architecture/architecture-templating-tools.graphmlimages/architecture/architecture_packages.graphmlpackages/blaze-tools/tojs.jspackages/blaze-tools/token_tests.jspackages/blaze-tools/tokens.jspackages/blaze/.npm/package/.gitignorepackages/blaze/.npm/package/READMEpackages/blaze/.npm/package/npm-shrinkwrap.jsonpackages/blaze/attrs.jspackages/blaze/builtins.jspackages/blaze/dombackend.jspackages/blaze/domrange.jspackages/blaze/events.jspackages/blaze/exceptions.jspackages/blaze/lookup.jspackages/blaze/materializer.jspackages/blaze/package.jspackages/blaze/preamble.jspackages/blaze/template.jspackages/blaze/utils.jspackages/blaze/view.jspackages/caching-html-compiler/.npm/package/.gitignorepackages/caching-html-compiler/.npm/package/READMEpackages/caching-html-compiler/.npm/package/npm-shrinkwrap.jsonpackages/caching-html-compiler/caching-html-compiler.jspackages/caching-html-compiler/package.jspackages/html-tools/charref.jspackages/html-tools/charref_tests.jspackages/html-tools/parse.jspackages/html-tools/parse_tests.jspackages/html-tools/scanner.jspackages/html-tools/templatetag.jspackages/html-tools/tokenize.jspackages/html-tools/tokenize_tests.jspackages/html-tools/utils.jspackages/htmljs/html.jspackages/htmljs/htmljs_test.jspackages/htmljs/visitors.jspackages/observe-sequence/observe_sequence.jspackages/observe-sequence/observe_sequence_tests.jspackages/spacebars-compiler/.npm/package/.gitignorepackages/spacebars-compiler/.npm/package/READMEpackages/spacebars-compiler/.npm/package/npm-shrinkwrap.jsonpackages/spacebars-compiler/codegen.jspackages/spacebars-compiler/compile_tests.jspackages/spacebars-compiler/compiler.jspackages/spacebars-compiler/optimizer.jspackages/spacebars-compiler/package.jspackages/spacebars-compiler/react.jspackages/spacebars-compiler/spacebars_tests.jspackages/spacebars-compiler/templatetag.jspackages/spacebars-compiler/whitespace.jspackages/spacebars-tests/old_templates.jspackages/spacebars-tests/old_templates_tests.jspackages/spacebars-tests/template_tests.jspackages/spacebars-tests/templating_tests.jspackages/spacebars/spacebars-runtime.jspackages/spacebars/spacebars_tests.jspackages/templating-runtime/.npm/package/.gitignorepackages/templating-runtime/.npm/package/READMEpackages/templating-runtime/.npm/package/npm-shrinkwrap.jsonpackages/templating-runtime/dynamic.jspackages/templating-runtime/dynamic_tests.jspackages/templating-runtime/package.jspackages/templating-runtime/templating.jspackages/templating-tools/.npm/package/.gitignorepackages/templating-tools/.npm/package/READMEpackages/templating-tools/.npm/package/npm-shrinkwrap.jsonpackages/templating-tools/compile-tags-with-spacebars.jspackages/templating-tools/html-scanner-tests.jspackages/templating-tools/html-scanner.jspackages/templating-tools/package.jstests/benchmark_ElementAttributesUpdater.htmltests/benchmark_getUrlProtocol.html
💤 Files with no reviewable changes (20)
- packages/caching-html-compiler/.npm/package/.gitignore
- packages/blaze/.npm/package/.gitignore
- packages/caching-html-compiler/.npm/package/npm-shrinkwrap.json
- packages/templating-tools/.npm/package/.gitignore
- packages/templating-runtime/.npm/package/.gitignore
- packages/spacebars-compiler/.npm/package/.gitignore
- packages/caching-html-compiler/package.js
- packages/spacebars-compiler/.npm/package/README
- packages/caching-html-compiler/.npm/package/README
- packages/templating-runtime/.npm/package/README
- packages/blaze/package.js
- packages/blaze/.npm/package/README
- packages/spacebars-compiler/package.js
- packages/blaze/.npm/package/npm-shrinkwrap.json
- packages/templating-runtime/.npm/package/npm-shrinkwrap.json
- packages/templating-tools/.npm/package/npm-shrinkwrap.json
- packages/templating-tools/.npm/package/README
- packages/spacebars-compiler/.npm/package/npm-shrinkwrap.json
- packages/templating-tools/package.js
- packages/templating-runtime/package.js
OVERVIEW.md
Outdated
| 2. **Reactive Runtime** - Manages DOM rendering and reactivity at runtime, responding to data changes automatically | ||
|
|
||
|  | ||
|  |
There was a problem hiding this comment.
Use a repository-relative image path instead of a branch-pinned raw URL.
Line 18 is brittle and can break when branch names or paths change. Prefer a relative path so docs render consistently across branches/forks.
📌 Suggested fix
-
+📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
|  | |
|  |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@OVERVIEW.md` at line 18, Replace the branch-pinned raw URL image link in the
markdown (the current
"https://raw.githubusercontent.com/meteor/blaze/architecture/images/architecture_packages.svg"
reference) with a repository-relative path to the asset (e.g. use the relative
Markdown image URL pointing at architecture/images/architecture_packages.svg) so
the docs render reliably across branches and forks; update the image markdown in
OVERVIEW.md accordingly.
Comment on lines
+199
to
+206
| ``` | ||
| .html file → templating-compiler | ||
| → caching-html-compiler | ||
| → templating-tools.scanHtmlForTags | ||
| → html-tools (parsing) | ||
| → spacebars-compiler (compilation) | ||
| → JavaScript code (output) | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks (MD040).
Lines 199, 209, and 384 use unlabeled fenced blocks; markdownlint flags this.
🧹 Suggested fix
-```
+```text
.html file → templating-compiler
→ caching-html-compiler
→ templating-tools.scanHtmlForTags
→ html-tools (parsing)
→ spacebars-compiler (compilation)
→ JavaScript code (output)- +text
Template function called
→ Creates Blaze.View
→ View.render() → HTMLjs structure
→ Blaze.materialize() → DOM nodes
→ Reactive dependencies tracked (Tracker)
→ Data changes → View.invalidate()
→ Selective DOM updates (observe-sequence for {{#each}})
-```
+```text
Template (Blueprint)
↓ constructView()
View (Low-level rendering primitive)
↑ referenced by
Template Instance (Specific instantiation, created alongside View)
↓ renders to
HTMLjs (Intermediate representation)
↓ materializes to
DOM Nodes (Actual browser elements)
</details>
Also applies to: 209-217, 384-394
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.0)</summary>
[warning] 199-199: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @OVERVIEW.md around lines 199 - 206, The unlabeled fenced code blocks flagged
by MD040 should each include a language identifier (use "text")—specifically add
block that begins "Template function called …", and the block that begins
"Template (Blueprint) …" so all three fenced blocks are labeled and markdownlint
MD040 is satisfied.
Comment on lines
+746
to
+762
| // Performance optimization: only update if value actually changed | ||
| // Values are always string or null (normalized by materializer), so strict equality suffices | ||
| const last = lastValues[k]; | ||
| const shouldUpdate = !(last === value || | ||
| (last == null && value == null)); | ||
|
|
||
| if (shouldUpdate) { | ||
| // Update the handler's stored value and apply to DOM | ||
| handler.value = value; | ||
| handler.update(elem, oldValue, value); | ||
| if (value === null) | ||
| lastValues[k] = value; | ||
|
|
||
| // Clean up if attribute was removed (value set to null) | ||
| if (value === null) { | ||
| delete handlers[k]; | ||
| delete lastValues[k]; | ||
| } |
There was a problem hiding this comment.
Don't short-circuit handler.update on cached values.
Line 749 skips handler.update based only on the last rendered value, but several handlers reconcile against the live element. After this change, rerendering the same template value no longer restores template-owned value/checked/class/style after user input or external DOM mutation.
Suggested fix
- const last = lastValues[k];
- const shouldUpdate = !(last === value ||
- (last == null && value == null));
-
- if (shouldUpdate) {
- // Update the handler's stored value and apply to DOM
- handler.value = value;
- handler.update(elem, oldValue, value);
- lastValues[k] = value;
-
- // Clean up if attribute was removed (value set to null)
- if (value === null) {
- delete handlers[k];
- delete lastValues[k];
- }
- }
+ handler.value = value;
+ handler.update(elem, oldValue, value);
+ lastValues[k] = value;
+
+ if (value === null) {
+ delete handlers[k];
+ delete lastValues[k];
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/blaze/attrs.js` around lines 746 - 762, The current short-circuit
uses shouldUpdate to skip calling handler.update when the new value equals
lastValues[k], which prevents handlers that reconcile against the live DOM
(e.g., for value/checked/class/style) from restoring template-owned state after
user/external mutation; change the logic in the block that references
lastValues, handlers, handler and shouldUpdate so that handler.update(elem,
oldValue, value) is always called regardless of shouldUpdate, but only update
handler.value and lastValues[k] (and perform the value === null cleanup that
deletes handlers[k] and lastValues[k]) when shouldUpdate is true — keep the same
calls and symbols (handler.update, handler.value, lastValues, handlers, value,
oldValue, elem) and only move the update invocation out of the short-circuit
guard.
Comment on lines
36
to
+42
| DOMBackend.parseHTML = function (html) { | ||
| // Return an array of nodes. | ||
| // | ||
| // jQuery does fancy stuff like creating an appropriate | ||
| // container element and setting innerHTML on it, as well | ||
| // as working around various IE quirks. | ||
| return $jq.parseHTML(html, DOMBackend.getContext()) || []; | ||
| if (_hasJQuery) { | ||
| return $jq.parseHTML(html, DOMBackend.getContext()) || []; | ||
| } | ||
| const template = document.createElement('template'); | ||
| template.innerHTML = html; | ||
| return Array.from(template.content.childNodes); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does jQuery.parseHTML(html, context) exclude <script> elements when keepScripts is omitted, and does setting template.innerHTML preserve <script> elements inside template.content?
💡 Result:
Yes, jQuery.parseHTML(html, context) excludes <script> elements when keepScripts is omitted (defaults to false), and yes, setting template.innerHTML preserves <script> elements inside template.content.
Citations:
- 1: https://api.jquery.com/jQuery.parseHTML/
- 2: https://github.com/jquery/jquery/blob/2.2-stable/src/core/parseHTML.js
- 3: https://api.jquery.com/jquery.parsehtml/
- 4: https://developer.mozilla.org/en-US/docs/Web/API/HTMLTemplateElement/content
- 5: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/template
- 6: https://stackoverflow.com/questions/35513437/template-tag-innerhtml-breaks-script-tags
Filter <script> nodes in the native parseHTML path.
Lines 37-42 no longer match the jQuery backend: $.parseHTML(...) excludes scripts by default, but the <template> branch includes them unchanged. This means the same HTML produces different DOM depending on whether jQuery is present, removing the script-filtering behavior that the old backend provided.
Suggested fix
DOMBackend.parseHTML = function (html) {
if (_hasJQuery) {
return $jq.parseHTML(html, DOMBackend.getContext()) || [];
}
const template = document.createElement('template');
template.innerHTML = html;
+ template.content.querySelectorAll('script').forEach((script) => script.remove());
return Array.from(template.content.childNodes);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| DOMBackend.parseHTML = function (html) { | |
| // Return an array of nodes. | |
| // | |
| // jQuery does fancy stuff like creating an appropriate | |
| // container element and setting innerHTML on it, as well | |
| // as working around various IE quirks. | |
| return $jq.parseHTML(html, DOMBackend.getContext()) || []; | |
| if (_hasJQuery) { | |
| return $jq.parseHTML(html, DOMBackend.getContext()) || []; | |
| } | |
| const template = document.createElement('template'); | |
| template.innerHTML = html; | |
| return Array.from(template.content.childNodes); | |
| DOMBackend.parseHTML = function (html) { | |
| if (_hasJQuery) { | |
| return $jq.parseHTML(html, DOMBackend.getContext()) || []; | |
| } | |
| const template = document.createElement('template'); | |
| template.innerHTML = html; | |
| template.content.querySelectorAll('script').forEach((script) => script.remove()); | |
| return Array.from(template.content.childNodes); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/blaze/dombackend.js` around lines 36 - 42, DOMBackend.parseHTML's
native branch currently returns template.content.childNodes which keeps <script>
elements, whereas the jQuery branch ($jq.parseHTML(...)) excludes scripts by
default; update the native path in DOMBackend.parseHTML to filter out SCRIPT
nodes before returning so both branches behave consistently. In practice, after
creating template and populating template.innerHTML, iterate over
template.content.childNodes (or use querySelectorAll) and return only nodes
whose nodeName !== 'SCRIPT' (or nodeType !== Node.ELEMENT_NODE || tagName !==
'SCRIPT') so script elements are excluded from the returned array.
Comment on lines
+94
to
+109
| undelegateEvents(elem, type, handler) { | ||
| if (_hasJQuery) { | ||
| $jq(elem).off(type, '**', handler); | ||
| return; | ||
| } | ||
|
|
||
| const handlerMap = _delegateMap.get(elem); | ||
| if (!handlerMap) return; | ||
|
|
||
| const entries = handlerMap.get(handler); | ||
| if (!entries) return; | ||
|
|
||
| for (const entry of entries) { | ||
| elem.removeEventListener(entry.eventType, entry.wrapper); | ||
| } | ||
| handlerMap.delete(handler); |
There was a problem hiding this comment.
Honor type when removing native delegated handlers.
Line 106 removes every stored wrapper for handler, regardless of type. If the same callback is delegated for multiple event types on one element, the first undelegateEvents call tears all of them down in native mode.
Suggested fix
const handlerMap = _delegateMap.get(elem);
if (!handlerMap) return;
const entries = handlerMap.get(handler);
if (!entries) return;
+ let eventType = DOMBackend.Events.parseEventType(type);
+ eventType = _delegateEventAlias[eventType] || eventType;
- for (const entry of entries) {
- elem.removeEventListener(entry.eventType, entry.wrapper);
- }
- handlerMap.delete(handler);
+ const remaining = [];
+ for (const entry of entries) {
+ if (entry.eventType === eventType) {
+ elem.removeEventListener(entry.eventType, entry.wrapper);
+ } else {
+ remaining.push(entry);
+ }
+ }
+ if (remaining.length) handlerMap.set(handler, remaining);
+ else handlerMap.delete(handler);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| undelegateEvents(elem, type, handler) { | |
| if (_hasJQuery) { | |
| $jq(elem).off(type, '**', handler); | |
| return; | |
| } | |
| const handlerMap = _delegateMap.get(elem); | |
| if (!handlerMap) return; | |
| const entries = handlerMap.get(handler); | |
| if (!entries) return; | |
| for (const entry of entries) { | |
| elem.removeEventListener(entry.eventType, entry.wrapper); | |
| } | |
| handlerMap.delete(handler); | |
| undelegateEvents(elem, type, handler) { | |
| if (_hasJQuery) { | |
| $jq(elem).off(type, '**', handler); | |
| return; | |
| } | |
| const handlerMap = _delegateMap.get(elem); | |
| if (!handlerMap) return; | |
| const entries = handlerMap.get(handler); | |
| if (!entries) return; | |
| let eventType = DOMBackend.Events.parseEventType(type); | |
| eventType = _delegateEventAlias[eventType] || eventType; | |
| const remaining = []; | |
| for (const entry of entries) { | |
| if (entry.eventType === eventType) { | |
| elem.removeEventListener(entry.eventType, entry.wrapper); | |
| } else { | |
| remaining.push(entry); | |
| } | |
| } | |
| if (remaining.length) handlerMap.set(handler, remaining); | |
| else handlerMap.delete(handler); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/blaze/dombackend.js` around lines 94 - 109, The undelegateEvents
implementation currently removes all wrappers for a given handler regardless of
the passed type; update undelegateEvents to only remove entries whose
entry.eventType equals the provided type: look up handlerMap via
_delegateMap.get(elem), get entries = handlerMap.get(handler), iterate and only
call elem.removeEventListener(entry.eventType, entry.wrapper) for entries where
entry.eventType === type (or matches the provided type string), remove those
matching entries from the entries collection, and only call
handlerMap.delete(handler) if no entries remain for that handler; reference
undelegateEvents, _delegateMap, handlerMap, entries, and
entry.eventType/entry.wrapper to locate the change.
Comment on lines
+34
to
37
| let pastInput = input.substring(pos - CONTEXT_AMOUNT - 1, pos); | ||
| if (pastInput.length > CONTEXT_AMOUNT) | ||
| pastInput = '...' + pastInput.substring(-CONTEXT_AMOUNT); | ||
| pastInput = `...${pastInput.substring(-CONTEXT_AMOUNT)}`; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify negative substring usage in scanner code.
rg -n -C2 --iglob '*scanner.js' 'substring\(\s*-\s*[A-Za-z0-9_$]+'Repository: meteor/blaze
Length of output: 465
🏁 Script executed:
node <<'EOF'
// Verify substring vs slice behavior with negative indices
const testString = "abcdefghij"; // 10 chars
const CONTEXT_AMOUNT = 3;
console.log("Test string:", testString);
console.log("Length:", testString.length);
console.log("\nWith CONTEXT_AMOUNT = 3:");
console.log("substring(-3):", testString.substring(-3));
console.log("slice(-3):", testString.slice(-3));
console.log("\nWith CONTEXT_AMOUNT = 5:");
console.log("substring(-5):", testString.substring(-5));
console.log("slice(-5):", testString.slice(-5));
// Simulate the actual code logic
const pastInput = "This is some context text that is longer than needed";
if (pastInput.length > CONTEXT_AMOUNT) {
const buggyVersion = `...${pastInput.substring(-CONTEXT_AMOUNT)}`;
const fixedVersion = `...${pastInput.slice(-CONTEXT_AMOUNT)}`;
console.log("\nActual code simulation:");
console.log("Original pastInput length:", pastInput.length);
console.log("Buggy version (substring):", buggyVersion);
console.log("Fixed version (slice):", fixedVersion);
}
EOFRepository: meteor/blaze
Length of output: 394
Replace substring(-CONTEXT_AMOUNT) with slice(-CONTEXT_AMOUNT) to properly truncate trailing context.
On line 36, substring(-CONTEXT_AMOUNT) doesn't truncate to the last N characters—it treats the negative index as 0 and returns the entire string. Use slice(-CONTEXT_AMOUNT) instead, which correctly returns the trailing N characters.
Proposed fix
- pastInput = `...${pastInput.substring(-CONTEXT_AMOUNT)}`;
+ pastInput = `...${pastInput.slice(-CONTEXT_AMOUNT)}`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/html-tools/scanner.js` around lines 34 - 37, The truncation logic
for pastInput is wrong because substring(-CONTEXT_AMOUNT) treats negative
indices as 0; in the block that sets pastInput (using variables input, pos,
CONTEXT_AMOUNT and the pastInput variable), replace the use of
substring(-CONTEXT_AMOUNT) with slice(-CONTEXT_AMOUNT) so pastInput =
`...${pastInput.slice(-CONTEXT_AMOUNT)}` to correctly return the trailing N
characters when pastInput.length > CONTEXT_AMOUNT.
Comment on lines
+25
to
+30
| const postProcess = function (string) { | ||
| // remove initial and trailing parens | ||
| string = string.replace(/^\(([\S\s]*)\)$/, '$1'); | ||
| if (! (Package['minifier-js'] && Package['minifier-js'].UglifyJSMinify)) { | ||
| // these tests work a lot better with access to beautification, | ||
| // but let's at least do some sort of test without it. | ||
| // These regexes may have to be adjusted if new tests are added. | ||
|
|
||
| // ======================== !!NOTE!! ================================= | ||
| // Since we are bringing uglify-js in from NPM, this code should no | ||
| // longer ever be needed. Leaving it just in case. | ||
| // ==================================+================================ | ||
|
|
||
| // Remove single-line comments, including line nums from build system. | ||
| string = string.replace(/\/\/.*$/mg, ''); | ||
| string = string.replace(/\s+/g, ''); // kill whitespace | ||
| } | ||
| // Remove single-line comments, including line nums from build system. | ||
| string = string.replace(/\/\/.*$/mg, ''); | ||
| string = string.replace(/\s+/g, ''); // kill whitespace |
There was a problem hiding this comment.
postProcess can erase // inside string literals and weaken test validity.
At Line [29], /\/\/.*$/mg strips from // to EOL indiscriminately. That can truncate literal content (for example http://...) and let distinct outputs compare equal.
🔧 Proposed fix
- string = string.replace(/\/\/.*$/mg, '');
+ // Strip only standalone comment lines injected by tooling.
+ string = string.replace(/^\s*\/\/.*$/mg, '');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const postProcess = function (string) { | |
| // remove initial and trailing parens | |
| string = string.replace(/^\(([\S\s]*)\)$/, '$1'); | |
| if (! (Package['minifier-js'] && Package['minifier-js'].UglifyJSMinify)) { | |
| // these tests work a lot better with access to beautification, | |
| // but let's at least do some sort of test without it. | |
| // These regexes may have to be adjusted if new tests are added. | |
| // ======================== !!NOTE!! ================================= | |
| // Since we are bringing uglify-js in from NPM, this code should no | |
| // longer ever be needed. Leaving it just in case. | |
| // ==================================+================================ | |
| // Remove single-line comments, including line nums from build system. | |
| string = string.replace(/\/\/.*$/mg, ''); | |
| string = string.replace(/\s+/g, ''); // kill whitespace | |
| } | |
| // Remove single-line comments, including line nums from build system. | |
| string = string.replace(/\/\/.*$/mg, ''); | |
| string = string.replace(/\s+/g, ''); // kill whitespace | |
| const postProcess = function (string) { | |
| // remove initial and trailing parens | |
| string = string.replace(/^\(([\S\s]*)\)$/, '$1'); | |
| // Remove single-line comments, including line nums from build system. | |
| // Strip only standalone comment lines injected by tooling. | |
| string = string.replace(/^\s*\/\/.*$/mg, ''); | |
| string = string.replace(/\s+/g, ''); // kill whitespace |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/spacebars-compiler/compile_tests.js` around lines 25 - 30, The
postProcess function currently uses the regex /\/\/.*$/mg which naively strips
from // to end-of-line and can remove // sequences inside string literals (e.g.,
"http://..."); replace that regex usage in postProcess with a safe line-by-line
comment stripper that scans each line character-by-character, tracking quote
state and escapes so it only removes // when not inside single or double quotes
(implement a helper like stripLineCommentsPreservingStrings used from
postProcess), then continue with the existing whitespace collapse and
paren-trimming steps; ensure the new code references postProcess and removes the
/\/\/.*$/mg usage.
Comment on lines
359
to
366
| TemplateTag.parseCompleteTag = function (scannerOrString, position) { | ||
| var scanner = scannerOrString; | ||
| let scanner = scannerOrString; | ||
| if (typeof scanner === 'string') | ||
| scanner = new HTMLTools.Scanner(scannerOrString); | ||
|
|
||
| var startPos = scanner.pos; // for error messages | ||
| var result = TemplateTag.parse(scannerOrString); | ||
| const startPos = scanner.pos; // for error messages | ||
| const result = TemplateTag.parse(scannerOrString); | ||
| if (! result) |
There was a problem hiding this comment.
Use the local scanner for parsing to avoid cursor desynchronization.
At Line [365], TemplateTag.parse(scannerOrString) bypasses the local scanner. For string input, this parses with a different scanner instance, while subsequent logic (parseFragment, validation, error positions) continues with scanner, producing inconsistent behavior.
🔧 Proposed fix
- const result = TemplateTag.parse(scannerOrString);
+ const result = TemplateTag.parse(scanner);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| TemplateTag.parseCompleteTag = function (scannerOrString, position) { | |
| var scanner = scannerOrString; | |
| let scanner = scannerOrString; | |
| if (typeof scanner === 'string') | |
| scanner = new HTMLTools.Scanner(scannerOrString); | |
| var startPos = scanner.pos; // for error messages | |
| var result = TemplateTag.parse(scannerOrString); | |
| const startPos = scanner.pos; // for error messages | |
| const result = TemplateTag.parse(scannerOrString); | |
| if (! result) | |
| TemplateTag.parseCompleteTag = function (scannerOrString, position) { | |
| let scanner = scannerOrString; | |
| if (typeof scanner === 'string') | |
| scanner = new HTMLTools.Scanner(scannerOrString); | |
| const startPos = scanner.pos; // for error messages | |
| const result = TemplateTag.parse(scanner); | |
| if (! result) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/spacebars-compiler/templatetag.js` around lines 359 - 366,
TemplateTag.parseCompleteTag is creating a local variable scanner but then calls
TemplateTag.parse(scannerOrString), which can create a different Scanner and
desynchronize cursor positions; change the call to use the local scanner
variable (i.e., call TemplateTag.parse(scanner)) so parsing, parseFragment,
validation and error position reporting all use the same Scanner instance
(update any other places in this function that pass scannerOrString into
internal parse functions to use scanner).
Comment on lines
161
to
164
| Template._migrateTemplate = function (templateName, newTemplate, migrate) { | ||
| var oldTemplate = Template[templateName]; | ||
| var migrate = Template.__pendingReplacement.indexOf(templateName) > -1 | ||
| const oldTemplate = Template[templateName]; | ||
| migrate = Template.__pendingReplacement.includes(templateName); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Definition context
rg -nP --type=js -C3 '\bTemplate\._migrateTemplate\s*=\s*function\s*\('
# 2) All call sites
rg -nP --type=js -C2 '\bTemplate\._migrateTemplate\s*\('
# 3) Calls passing 3 arguments (potentially impacted if arg is ignored)
rg -nP --type=js -C2 '\bTemplate\._migrateTemplate\s*\(([^,]*,){2}[^)]*\)'Repository: meteor/blaze
Length of output: 957
🏁 Script executed:
cat -n packages/templating-tools/code-generation.js | sed -n '1,20p'Repository: meteor/blaze
Length of output: 833
🏁 Script executed:
# Look at the complete call in code-generation.js
cat -n packages/templating-tools/code-generation.js | sed -n '7,15p'Repository: meteor/blaze
Length of output: 371
🏁 Script executed:
# Search for any other Template._migrateTemplate calls more broadly
rg -n '_migrateTemplate' packages/Repository: meteor/blaze
Length of output: 362
Remove unused migrate parameter from _migrateTemplate.
The migrate parameter is never passed by callers. The only call site in code-generation.js:9-12 provides only 2 arguments (templateName and newTemplate), making the parameter dead code. Either remove it or, if it's intended as an optional override, change line 163 to migrate = migrate || Template.__pendingReplacement.includes(templateName); to respect caller input when provided.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/templating-runtime/templating.js` around lines 161 - 164, The
function signature for Template._migrateTemplate currently declares an unused
third parameter `migrate`; remove it and compute migration intent locally
instead so caller input isn't expected. Change the signature to
Template._migrateTemplate = function (templateName, newTemplate) { and replace
the reassignment line with a local const, e.g. const migrate =
Template.__pendingReplacement.includes(templateName); (or delete any unused
`migrate` variable entirely if it’s not referenced later).
Comment on lines
+5
to
+6
| <a href="https://github.com/meteor/blaze/actions/workflows/blaze-tests.yml"> | ||
| <img src="https://github.com/meteor/blaze/actions/workflows/blaze-tests.yml/badge.svg"> |
There was a problem hiding this comment.
Add alt text to the GitHub Actions badge for accessibility.
The badge image is missing alternative text, which impacts accessibility for screen reader users.
♿ Proposed fix to add descriptive alt text
<a href="https://github.com/meteor/blaze/actions/workflows/blaze-tests.yml">
- <img src="https://github.com/meteor/blaze/actions/workflows/blaze-tests.yml/badge.svg">
+ <img src="https://github.com/meteor/blaze/actions/workflows/blaze-tests.yml/badge.svg" alt="Blaze Tests">
</a>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <a href="https://github.com/meteor/blaze/actions/workflows/blaze-tests.yml"> | |
| <img src="https://github.com/meteor/blaze/actions/workflows/blaze-tests.yml/badge.svg"> | |
| <a href="https://github.com/meteor/blaze/actions/workflows/blaze-tests.yml"> | |
| <img src="https://github.com/meteor/blaze/actions/workflows/blaze-tests.yml/badge.svg" alt="Blaze Tests"> | |
| </a> |
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 6-6: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 5 - 6, The badge image tag lacks alternative text;
update the README.md badge <img> element (inside the <a
href="https://github.com/meteor/blaze/actions/workflows/blaze-tests.yml"> link)
to include a descriptive alt attribute (e.g., alt="GitHub Actions — blaze-tests
workflow status") so screen readers can convey the badge meaning; ensure the alt
text is concise and specific to the workflow name/status.
* docs: migrate to vitepress * docs: replace soft links with hard links * docs: update vitepress Theme and Nav items * docs: fix code syntax highlighting * ci: add deployment workflow * ci: fix paths * ci: remove failing cache * ci: fix paths * docs: update Blaze api * docs: add react guide from old v2 Meteor docs to Blaze docs * docs: include architecture overview in development guide * fix(docs): use hard link for repo files * docs: generate api docs from jsDoc comments * fix(docs): api links fixed * docs: add public folder with assets to vitepress site
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is our ongoing development toward the upcoming 3.1.0 release.
Merged PRs
Resolved Issues
Release
Summary by CodeRabbit
New Features
Improvements
Tests