Remove jQuery from answer types #2278
Conversation
|
Size Change: -22 B (0%) Total Size: 735 kB
ℹ️ View Unchanged
|
| return []; | ||
| }); | ||
| const transformed = fractionTransformer(text); | ||
| return transformed.reduce( |
There was a problem hiding this comment.
jQuery's map seemingly does some special flattening/filtering that's not as straight-forward as JS' map: HubSpot/youmightnotneedjquery#355
There was a problem hiding this comment.
Agreed. jQuery's map says for the mapping callback:
The function can return:
* the translated value, which will be mapped to the resulting array
* null or undefined, to remove the item
* an array of values, which will be flattened into the full array
That last point comes into play. I think you could just use flatMap() here instead of reduce()
There was a problem hiding this comment.
Good call out, thank you
| @@ -442,17 +461,17 @@ const KhanAnswerTypes = { | |||
|
|
|||
| // Numbers with percent signs | |||
| percent: function (text) { | |||
| text = $.trim(text); | |||
| text = String(text).trim(); | |||
There was a problem hiding this comment.
jQuery's trim coerces numbers to strings and apparently our code was banking on that.
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (ae6ed1f) and published it to npm. You Example: pnpm add @khanacademy/perseus@PR2278If you are working in Khan Academy's webapp, you can run: ./dev/tools/bump_perseus_version.sh -t PR2278 |
handeyeco
changed the title
handeyeco
added
🐍 project: sss
| return []; | ||
| }); | ||
| const transformed = fractionTransformer(text); | ||
| return transformed.reduce( |
There was a problem hiding this comment.
Agreed. jQuery's map says for the mapping callback:
The function can return:
* the translated value, which will be mapped to the resulting array
* null or undefined, to remove the item
* an array of values, which will be flattened into the full array
That last point comes into play. I think you could just use flatMap() here instead of reduce()
Summary:
I'm kind of just trying this out, but I think it would be good to remove jQuery where possible anyway.
See: https://khanacademy.slack.com/archives/CDHPTMSTB/p1741115861977089