Add JetStreamParams#171
Conversation
✅ Deploy Preview for webkit-jetstream-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
side-note: once we have this, I can also copy over the dev-menu from speedometer, which makes these parameters a bit more accessible. |
danleh
left a comment
danleh
left a comment
There was a problem hiding this comment.
Overall LGTM, some nits.
The high-level motivation (besides adding a UI menu in the browser later) seems to be (i) refactoring/cleanup and (ii) allowing detection of non-standard parameters, such that we can warn somehow when displaying results.
| console.log(" ", tagName); | ||
| console.log(""); | ||
|
|
||
| if (typeof benchmarksByTag !== "undefined") { |
There was a problem hiding this comment.
Why this check, isn't benchmarksByTag always defined in
Line 2634 in f8e3d7e
In that case, can this just be an assert?
| constructor(sourceParams = undefined) { | ||
| if (sourceParams) | ||
| this._copyFromSearchParams(sourceParams); | ||
| if (!this.developerMode) |
There was a problem hiding this comment.
You removed developer mode from the CLI options above, but not here. Is this intended? I'm still not sure I got the intuition behind the developer mode, how is it different / what does it ensure additionally on top of just warning when folks run JetStream with non-default parameters?
2 participants
For displaying a warning about non-default params we need a way to keep track of what the default values are.
We can reuse the existing code from Speedometer which does exactly that.