Adds Danger.parseFloat, System.sampleCount, Danger.now #2974
Conversation
🦋 Changeset detectedLatest commit: db8f17b The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
| @@ -23,6 +23,10 @@ describe("Danger functions", () => { | |||
| testEvalToBe("Danger.allCombinations([3])", "[[3]]"); | |||
| testEvalToBe("Danger.allCombinations([])", "[]"); | |||
| }); | |||
| describe("javascript", () => { | |||
| testEvalToBe("Danger.parseFloat('10.0') + 1", "11"); | |||
| testEvalToBe("Danger.parseFloat('foo')", '"Parse Failed"'); | |||
There was a problem hiding this comment.
Wasn't sure what the best error string was, because it's in Danger, we don't need to worry about quite as much.
There was a problem hiding this comment.
In the future, parseFloat("str").unwrap() could be good; for that, we'd need:
- some kind of GADTs or structs
- ideally, methods (because calling
-> Result.unwrapis too verbose) - exception handling
| import { ImmutableMap } from "../utility/immutableMap.js"; | ||
| import { Value, vString } from "../value/index.js"; | ||
|
|
||
| // automatically updated on release by ops/ patch-js utils |
There was a problem hiding this comment.
TODO: I realized we'd need to change patch-js
There was a problem hiding this comment.
Can you just move it back to version.ts for now? I could update patch-js, but it's better if the patched file is small, because it will be reformatted on patch and Babel tends to ignore the whitespace.
There was a problem hiding this comment.
Danger.parseFloat and System.sampleCount seem good to me, but I'm not happy about Date.now.
I probably shouldn't push back on it, because it's immediately useful, but it makes Squiggle runs less reproducible, and later it will interfere with caching and I predict we'll have to spend a lot of time untangling the consequences on this change.
The complexity of how to combine reproducible runs with mutable inputs is not trivial either way, and it's obvious we do want mutable inputs. So the question is whether we should think through the design now or later.
| @@ -23,6 +23,10 @@ describe("Danger functions", () => { | |||
| testEvalToBe("Danger.allCombinations([3])", "[[3]]"); | |||
| testEvalToBe("Danger.allCombinations([])", "[]"); | |||
| }); | |||
| describe("javascript", () => { | |||
| testEvalToBe("Danger.parseFloat('10.0') + 1", "11"); | |||
| testEvalToBe("Danger.parseFloat('foo')", '"Parse Failed"'); | |||
There was a problem hiding this comment.
In the future, parseFloat("str").unwrap() could be good; for that, we'd need:
- some kind of GADTs or structs
- ideally, methods (because calling
-> Result.unwrapis too verbose) - exception handling
| import { ImmutableMap } from "../utility/immutableMap.js"; | ||
| import { Value, vString } from "../value/index.js"; | ||
|
|
||
| // automatically updated on release by ops/ patch-js utils |
There was a problem hiding this comment.
Can you just move it back to version.ts for now? I could update patch-js, but it's better if the patched file is small, because it will be reformatted on patch and Babel tends to ignore the whitespace.
| }); | ||
|
|
||
| export const library = [ | ||
| // It might make sense to later make this a constant, as this number shouldn't change at runtime. |
There was a problem hiding this comment.
Yes, but you can't just add it to stdlib, which is cached in SqProject.
I guess you could convert getStdLib() to getStdLib(env: Environment), and then invalidate it in SqProject on setEnvironment...
OAGr
changed the title
Adds 3 Std Lib functions. Also adds a Docs page for System