Match nprogress timing more closely - #3025
Merged
Merged
Conversation
The comment claimed the original nprogress `done()` includes a random progress jump that we omit, implying a visible divergence. The jump exists in the original's code but never renders: its queue shifts each callback out of `pending` before running it, so the jump and the go-to-1 step both execute in the same tick and the second CSS write overwrites the first before paint. Measured side by side, both complete in identical fill and fade windows. Also drop the suggestion that consumers can set a higher progress value before stopping the animation. The hook exposes no progress setter, so that guidance cannot be followed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Running a Next example's dev server leaves generated output in examples/*/.next. It is gitignored per example, but neither prettier nor eslint reads those files, so a subsequent `npm test` at the repo root failed in check:format and lint on thousands of generated-file problems. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The original nprogress `start()` calls set(0), which clamps to `minimum`, so the bar first paints at 0.08 and the first two trickles are +0.1 each: 0.08, 0.18, 0.28. Our `start` action used increment(0) = 0.1, so the second trickle crossed into the +0.04 tier immediately: 0.1, 0.2, 0.24. Measured side by side against nprogress master, the two curves ran 2-4 points apart for the first few seconds. Set the start progress to clamp(0, minimum, 1) instead. Verified in a browser against the real nprogress master build: the sampled values now match at every trickle. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Trickle cadence and easing were already configurable, but step size was not, so curves like npm nprogress 0.2.0's random amount of at most 0.02 could not be expressed. `increment` takes the current progress and returns the next value, defaulting to the existing tiered curve. The reducer still clamps the result between `minimum` and `1`, so a custom function cannot move the bar outside the documented range. Any ceiling short of 1 belongs to that function. The trickle timer reads the function through a ref rather than a dependency. Consumers commonly pass an inline function, and depending on its identity would cancel and recreate the timer on every render, stalling the bar whenever renders outpace `incrementDuration`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The npm 0.2.0 release and the nprogress master branch trickle differently, and the official demo page loads 0.2.0. Say in the background section that this library follows master, and point at the increment option for the older pacing. Note the easing difference alongside the 0.2.0 increment example, since easing is the renderer's CSS rather than an option here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`progress` returns to `minimum` when the bar starts again, so a bar left mounted between runs transitions backwards from where it finished, in full view. Every example already avoids this by changing a key on the progress component at each start, but only the react-router example said why, and the README usage snippets do not show the pattern at all. Add a "Restarting" note to the README covering the behaviour and the keying pattern, and a comment at each key flip in the examples. Dropping the transition while `isFinished` looks like an alternative but is not: the commit that resets `progress` also flips `isFinished` to `false`, so the transition is live for the step that moves the bar.
The npm 0.2.0 release is what the nprogress demo page loads, and its pacing is the best-known one: a random amount of at most 0.02 every 800ms, eased. The defaults here follow the master branch instead, so the `increment` option is what closes the gap. Configuration mirrors 0.2.0's trickleRate, trickleSpeed and easing. The remaining settings already match: minimum 0.08, and speed 200, which is animationDuration. The bar and spinner styling is the original-design example's, so the only visible difference between the two is pacing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to a timing investigation comparing v7 against the original nprogress. Two versions of nprogress are in circulation and they trickle differently: the 2014 npm release
0.2.0, which the official demo page loads, and the master branch, never published to npm. This library mirrors master. That was undocumented, and one ingredient of the0.2.0feel was not reachable through the options.Changes
done(). The original computes a random progress jump before animating to 1, but its queue runs both steps in the same tick, so the jump never renders. Dropping it loses nothing visually.minimumrather thanincrement(0), matching the original'sstart(), which callsset(0). Measured side by side the curves ran 2-4 points apart early because of this.incrementoption, defaulting to the current tiered curve. Cadence and easing were already configurable, so step size was the last thing fixed. The return value is clamped to betweenminimumand1and nothing else, so a custom function owns its own ceiling.0.2.0pacing.classic-020example reproducing0.2.0:trickleRateas anincrementfunction,trickleSpeedasincrementDuration, andeasingin the renderer's CSS.Notes
incrementis additive, so this is a minor. The trickle timer holds the function in a ref and does not list it as a dependency, so passing an inline function does not restart the timer.Verification
npm testpasses, coverage stays at 100%. Theclassic-020example was smoke-tested against a packed tarball rather than the registry, then driven in a browser: measured cadence ~820ms, random steps of at most 0.02. Run against the real0.2.0in a four-lane comparison page it matched on mean cadence (800ms against 819ms) and mean step (0.0106 against 0.0093).Outstanding: the
classic-020example still needs a CodeSandbox check, which has to wait for the release that putsincrementon the registry. Examples resolvelatest.🤖 Generated with Claude Code