Skip to content

config endpoint must handle functions in module configs#4106

Merged
sdetweil merged 2 commits intoMagicMirrorOrg:developfrom
khassel:config
Apr 12, 2026
Merged

config endpoint must handle functions in module configs#4106
sdetweil merged 2 commits intoMagicMirrorOrg:developfrom
khassel:config

Conversation

@khassel
Copy link
Copy Markdown
Collaborator

@khassel khassel commented Apr 12, 2026

Fixes #4105

In JavaScript, standard JSON does not support functions.
If you use JSON.stringify() on an object containing functions,
those functions will be omitted (if they are object properties)
or changed to null (if they are in an array).

@KristjanESPERANTO
Copy link
Copy Markdown
Collaborator

Smart solution! I noticed some small polish points around the function-tagging flow on the server/client side (plus one small edge-case test for plain strings).

Would you be okay with me adding a follow-up commit to your branch? Would be easier than commenting.

@khassel
Copy link
Copy Markdown
Collaborator Author

khassel commented Apr 12, 2026

Sure

Replace heuristic string detection for function revival with an
explicit __mmFunction tag to avoid false positives for plain text
containing "function" or "=>".

Add comments clarifying the server/client serialization contract and
extend e2e coverage to ensure normal strings are not revived.
@KristjanESPERANTO
Copy link
Copy Markdown
Collaborator

Just pushed a commit: functions are now explicitly marked during serialization instead of guessed, which should make the flow safer and prevents false positives on plain strings.

What do you think?

@khassel
Copy link
Copy Markdown
Collaborator Author

khassel commented Apr 12, 2026

I'm fine with this. I'm not a JavaScript expert anyway, so I would never have come up with that idea...

Since we both worked on this, it should be merged by @rejas or @sdetweil .

@sdetweil sdetweil merged commit d3a3ad9 into MagicMirrorOrg:develop Apr 12, 2026
12 checks passed
@khassel khassel deleted the config branch April 12, 2026 21:06
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.

3 participants