Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass standard “Array.map” arguments to “repeat”. #287

Merged
merged 1 commit into from
Mar 9, 2025

Conversation

theengineear
Copy link
Collaborator

The “repeat” directive is just a shim to create arrays of entries. We may remove this from the interface in the future, but we do need it to function as expected in the interim!

Most callers rely on just the first “value” argument, but there’s no good reason not to expose the (value, index, array)-triple that a standard callback would receive from “Array.map”.

The “repeat” directive is just a shim to create arrays of _entries_. We
may remove this from the interface in the future, but we do need it
to function as expected in the interim!

Most callers rely on just the first “value” argument, but there’s no
good reason not to expose the `(value, index, array)`-triple that a
standard callback would receive from “Array.map”.
@@ -1241,6 +1241,38 @@ describe('html updaters', () => {
assert(container.querySelector('#c').textContent === 'fuzz');
container.remove();
});

it('repeat callbacks are provided args from underlying “.map” call', () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

FYI —Added this test first to prove that we could repro failure in our test suite. Then, added the related fix.

@@ -126,8 +126,8 @@ class TemplateEngine {
throw new Error(`Unexpected repeat callback "${callback}" provided, expected a function.`);
}
return identify
? items.map(item => [identify(item), callback(item)])
: items.map(item => callback(item)); // Just a basic array.
? items.map((...args) => [identify(...args), callback(...args)])
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Super simple fix to just pass everything through as-is.

@theengineear
Copy link
Collaborator Author

FYI @klebba — noticed this one while I was working today. It was a quick fix, so I figured I’d throw in a quick update.

@theengineear theengineear merged commit feb1382 into main Mar 9, 2025
1 check passed
@theengineear theengineear deleted the fix-repeat-arguments branch March 9, 2025 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant