Skip to content
This repository has been archived by the owner on Nov 29, 2023. It is now read-only.

Prevent conversion of native number params to Scientific Notation #160

Merged
merged 2 commits into from Sep 16, 2022

Conversation

jkuester
Copy link
Contributor

Updates the processing of native function parameters to set a fix number of fractional digits (20) to include after the decimal point when appending the number value to the native function string. This will prevent JavaScript from representing numbers with many fractional digits using Scientific Notation (which causes an error in some native XPath functions).

I believe that currently only the native ceiling and floor functions are affected since they are the only supported native functions that accept number parameters.

Closes enketo/enketo#863

@jkuester jkuester changed the title Prevent conversion of number parameters to Scientific Notation when calling native functions Prevent conversion of native number params to Scientific Notation Jul 11, 2022
Copy link
Contributor

@garethbowen garethbowen left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@alxndrsn Seeing as you commented on the issue, what do you think?

@garethbowen
Copy link
Contributor

@yanokwa Do you have a minute to review this change before we merge?

@alxndrsn
Copy link
Contributor

@garethbowen LGTM. I think it also closes enketo/enketo#896

@garethbowen
Copy link
Contributor

@yanokwa @eyelidlessness I think this is good to go. Are you happy for this to be merged, and a new release being published? We're hoping to include this in a release in the next couple of weeks.

@lognaturel
Copy link
Contributor

Happy for it to be merged and released. Do you also need a core release to be able to use it? We've been heads down in Express and weren't planning to release the other tools for another 3ish weeks but could possibly revise that.

@jkuester
Copy link
Contributor Author

@lognaturel no, we should not need enketo-core to be released. The new version of openrosa should be compatible with our current version of enketo-core.

@garethbowen
Copy link
Contributor

@lognaturel I'm happy to do the merge and release, but I don't think I have the npm permissions required to do the actual publish. Would you mind helping out either by doing the publish, or granting me permissions on npm?

@lognaturel lognaturel merged commit 4316e52 into enketo:master Sep 16, 2022
@lognaturel
Copy link
Contributor

Will try to do by middle of next week.

@lognaturel
Copy link
Contributor

It's out.

@jkuester
Copy link
Contributor Author

Thanks a ton @lognaturel! This is very helpful! We will be pulling in the new release this week.

@jkuester jkuester deleted the 159-fix-native-numbers branch September 17, 2022 13:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Numbers represented by JS with Scientific Notation break native xPath functions
4 participants