Skip to content

Commit 66bc9bc

Browse files
authored
Merge pull request #3006 from tanem/logic
Align logic more closely with the original nprogress, add React boundary tests
2 parents b526ee1 + b6d9a86 commit 66bc9bc

22 files changed

Lines changed: 447 additions & 26 deletions

.github/copilot-instructions.md

Lines changed: 50 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ changes agent behaviour and cannot be inferred from the codebase or tooling.
77

88
TypeScript React library providing a slim progress bar primitive via three
99
patterns: `useNProgress` hook, `NProgress` render-props component, and
10-
`withNProgress` HOC. Exports logic only — no rendering. All exports go through
10+
`withNProgress` HOC. Exports logic only, not rendering. All exports go through
1111
`src/index.tsx`. Types live in `src/types.ts`.
1212

1313
## Key Commands
@@ -22,14 +22,15 @@ npm run format # fix lint and formatting
2222

2323
### Comments
2424

25-
- Use `//` line comments only never `/* */` or `/** */`
25+
- Use `//` line comments only, never `/* */` or `/** */`
2626
- Explain _why_, not _what_; wrap at 80 characters
27+
- End every comment with a full stop, even single-line comments
2728

2829
### Language
2930

3031
Use **New Zealand English** in all user-facing text, variable names, and
3132
comments (e.g. "colour", "behaviour", "organisation"). Standardised API names
32-
(`color`, `textAlign`) are fixed leave them unchanged.
33+
(`color`, `textAlign`) are fixed: leave them unchanged.
3334

3435
```javascript
3536
const progressColour = '#0066cc'
@@ -49,16 +50,42 @@ subject, no trailing period, blank line between subject and body.
4950

5051
Managed by Renovate (`config:js-lib` preset):
5152

52-
- `devDependencies` pinned exact versions (no `^` or `~`)
53-
- `dependencies` caret ranges (`^`)
54-
- `peerDependencies` explicit OR ranges (e.g. `^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0`)
53+
- `devDependencies`: pinned exact versions (no `^` or `~`)
54+
- `dependencies`: caret ranges (`^`)
55+
- `peerDependencies`: explicit OR ranges (e.g. `^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0`)
5556
- Do **not** add `allowedVersions` to `renovate.json` without a documented reason
5657

5758
## Testing
5859

5960
- **100% code coverage** required across all build formats
6061
- Always run `npm test` after changes; use `npm run test:src` for quick
6162
source-only feedback during development
63+
- Use `npm run test:react` for the full React version matrix independently.
64+
It also runs as part of `npm test` (via the `test:*` glob).
65+
66+
### React version matrix
67+
68+
We test boundary versions only: first and last minor of each supported
69+
major. See `test/react/` for current versions.
70+
71+
Current boundaries: 16.14, 17.0, 18.0, 18.3, 19.0.
72+
73+
React 16.14 is the practical lower bound. Hooks require 16.8 and
74+
`@testing-library/react-hooks` requires 16.9.
75+
76+
When adding a new boundary:
77+
78+
1. Add `test/react/<version>/package.json` with correct `react`,
79+
`react-dom`, and `@testing-library/react` (12.x for React 16–17,
80+
16.x for React 18+). React 16–17 also need
81+
`@testing-library/react-hooks` (8.x) and `react-test-renderer`.
82+
2. Replace the previous "latest minor" for that major.
83+
3. Verify with a single-version run before the full matrix:
84+
```bash
85+
cd test/react/<version> && npm i --no-package-lock --quiet --no-progress
86+
REACT_VERSION=<version> npx jest --config ./scripts/jest/config.src.js --coverage false
87+
```
88+
4. Update the boundary list above.
6289

6390
## Examples
6491

@@ -85,7 +112,23 @@ needed but test on CodeSandbox before merging.
85112
Do not bump vite, @vitejs/plugin-react, next, or typescript in examples
86113
beyond the versions in the reference templates.
87114

115+
## Writing Style
116+
117+
- Avoid marketing or promotional language. State facts plainly.
118+
- Follow best practices for technical writing: be clear, direct, and
119+
concise.
120+
- Avoid em dashes. Use colons, commas, or separate sentences instead.
121+
- Use present tense and active voice where practical.
122+
- Keep sentences short. One idea per sentence.
123+
88124
## Versioning
89125

90-
Strict semver no breaking changes without a major version bump, including
126+
Strict semver: no breaking changes without a major version bump, including
91127
technical refactors.
128+
129+
## Documentation
130+
131+
- After each code change, update all related docs and markdown files
132+
(README.md, MIGRATION.md, example READMEs, etc.) in the same pass.
133+
- Do not manually modify CHANGELOG.md. It is auto-generated during
134+
release.

MIGRATION.md

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,35 @@
11
# Migrating
22

3+
## v6.0.0
4+
5+
Trickle pacing was adjusted to more closely match the original
6+
[nprogress](https://github.com/rstacruz/nprogress) behaviour.
7+
8+
### `incrementDuration` default changed from `800` to `200`
9+
10+
The previous default of `800` meant each trickle took roughly one second
11+
(animation wait plus increment delay). The new default of `200` matches
12+
the original library's `trickleSpeed` and results in faster trickle
13+
pacing.
14+
15+
**Action required:** if you were relying on the old default pacing,
16+
explicitly pass `incrementDuration={800}` to restore the previous
17+
behaviour.
18+
19+
### Intermediate progress updates no longer wait for `animationDuration`
20+
21+
Non-completion `set()` calls previously waited `animationDuration`
22+
(200 ms) before advancing the internal queue. This wait has been
23+
removed for intermediate updates so that only `incrementDuration`
24+
controls trickle pacing. The completion path (`set(1)`) still waits
25+
`animationDuration` before marking the bar as finished, giving
26+
consumers time to animate the bar to 100% before it disappears.
27+
28+
**Action required:** none in most cases. If your rendering code relied
29+
on intermediate progress updates being spaced at least
30+
`animationDuration` apart, you may need to adjust your
31+
transition/animation timing.
32+
333
## v5.0.0
434

535
The prop-types package is no longer required for using the UMD builds.

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ render(<Enhanced isAnimating />, document.getElementById('root'))
111111
**Props**
112112

113113
- `animationDuration` - _Optional_ Number indicating the animation duration in `ms`. Defaults to `200`.
114-
- `incrementDuration` - _Optional_ Number indicating the length of time between progress bar increments in `ms`. Defaults to `800`.
114+
- `incrementDuration` - _Optional_ Number indicating the length of time between progress bar increments in `ms`. Defaults to `200`.
115115
- `isAnimating` - _Optional_ Boolean indicating if the progress bar is animating. Defaults to `false`.
116116
- `minimum` - _Optional_ Number between `0` and `1` indicating the minimum value of the progress bar. Defaults to `0.08`.
117117

package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
"clean:compiled": "shx rm -rf compiled",
2121
"clean:coverage": "shx rm -rf coverage",
2222
"clean:dist": "shx rm -rf dist",
23+
"clean:react": "node ./scripts/clean-react.js",
2324
"compile": "tsc -p tsconfig.base.json",
2425
"format": "npm run lint -- --fix && prettier --write \"**/*.{js,ts,tsx}\"",
2526
"lint": "eslint .",
@@ -30,6 +31,7 @@
3031
"test:cjs": "jest --config ./scripts/jest/config.cjs.js",
3132
"test:cjsprod": "jest --config ./scripts/jest/config.cjsprod.js",
3233
"test:es": "jest --config ./scripts/jest/config.es.js",
34+
"test:react": "node ./scripts/test-react.js",
3335
"test:src": "jest --config ./scripts/jest/config.src.js",
3436
"test:umd": "jest --config ./scripts/jest/config.umd.js",
3537
"test:umdprod": "jest --config ./scripts/jest/config.umdprod.js",

scripts/clean-react.js

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// Cleans node_modules from each test/react/<version> directory while preserving
2+
// the package.json file.
3+
const fs = require('fs')
4+
const os = require('os')
5+
const path = require('path')
6+
7+
const reactDir = path.join(process.cwd(), 'test', 'react')
8+
9+
fs.readdirSync(reactDir, { withFileTypes: true })
10+
.filter((entry) => entry.isDirectory())
11+
.forEach((entry) => {
12+
const dir = path.join(reactDir, entry.name)
13+
const pkgJsonPath = path.join(dir, 'package.json')
14+
15+
if (!fs.existsSync(pkgJsonPath)) {
16+
return
17+
}
18+
19+
// Preserve package.json by copying to a temporary location.
20+
const tmpPath = path.join(os.tmpdir(), `${entry.name}-package.json`)
21+
fs.copyFileSync(pkgJsonPath, tmpPath)
22+
fs.rmSync(dir, { force: true, recursive: true })
23+
fs.mkdirSync(dir, { recursive: true })
24+
fs.copyFileSync(tmpPath, pkgJsonPath)
25+
})

scripts/jest/config.src.js

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,74 @@
1+
const path = require('path')
2+
3+
// When REACT_VERSION is set (e.g. "18.0"), resolve react, react-dom and
4+
// @testing-library/react from the matching test/react/<version> directory so
5+
// the test suite runs against that specific React version.
6+
const generateReactVersionMappings = (reactVersion) => {
7+
if (!reactVersion) {
8+
return {}
9+
}
10+
11+
const testDir = path.join(process.cwd(), 'test', 'react', reactVersion)
12+
const [major] = reactVersion.split('.').map(Number)
13+
14+
const reactDir = path.dirname(
15+
require.resolve('react/package.json', { paths: [testDir] }),
16+
)
17+
const reactDomDir = path.dirname(
18+
require.resolve('react-dom/package.json', { paths: [testDir] }),
19+
)
20+
21+
const mappings = {
22+
'^react$': require.resolve('react', { paths: [testDir] }),
23+
'^react-dom$': require.resolve('react-dom', { paths: [testDir] }),
24+
'^react-dom/(.*)$': `${reactDomDir}/$1`,
25+
'^react/(.*)$': `${reactDir}/$1`,
26+
}
27+
28+
// React 16 and 17 use @testing-library/react 12.x which does not export
29+
// renderHook. A shim re-exports it from @testing-library/react-hooks instead.
30+
if (major < 18) {
31+
mappings['^@testing-library/react$'] = path.join(
32+
testDir,
33+
'..',
34+
'testing-library-shim.js',
35+
)
36+
} else {
37+
mappings['^@testing-library/react$'] = require.resolve(
38+
'@testing-library/react',
39+
{ paths: [testDir] },
40+
)
41+
}
42+
43+
return mappings
44+
}
45+
46+
// React 16 and 17 boundary tests produce harmless "wrong act()" warnings from
47+
// @testing-library/react-hooks. A setup file suppresses them.
48+
const generateSetupFiles = (reactVersion) => {
49+
if (!reactVersion) {
50+
return []
51+
}
52+
53+
const [major] = reactVersion.split('.').map(Number)
54+
if (major < 18) {
55+
return [path.join(__dirname, 'setupReact.js')]
56+
}
57+
58+
return []
59+
}
60+
161
module.exports = {
262
collectCoverage: true,
363
collectCoverageFrom: ['src/*.{ts,tsx}'],
464
moduleFileExtensions: ['ts', 'tsx', 'js'],
65+
moduleNameMapper: {
66+
...generateReactVersionMappings(process.env.REACT_VERSION),
67+
},
568
preset: 'ts-jest',
669
rootDir: process.cwd(),
770
roots: ['<rootDir>/test'],
71+
setupFilesAfterEnv: generateSetupFiles(process.env.REACT_VERSION),
872
testEnvironment: 'jsdom',
973
testMatch: ['<rootDir>/test/*.spec.ts?(x)'],
1074
transform: { '^.+\\.(js|tsx?)$': 'ts-jest' },

scripts/jest/setupReact.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// Suppresses the "wrong act()" console.error warnings that
2+
// @testing-library/react-hooks emits on React 16 and 17. The package uses
3+
// react-test-renderer internally while state updates flow through react-dom,
4+
// triggering a harmless mismatch warning.
5+
const originalError = console.error
6+
7+
console.error = (...args) => {
8+
if (
9+
typeof args[0] === 'string' &&
10+
args[0].includes("It looks like you're using the wrong act()")
11+
) {
12+
return
13+
}
14+
originalError.call(console, ...args)
15+
}

scripts/test-react.js

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Installs dependencies and runs the src test suite against each React version
2+
// defined in test/react/<version>.
3+
const { execSync } = require('child_process')
4+
const fs = require('fs')
5+
const path = require('path')
6+
7+
const reactDir = path.join(process.cwd(), 'test', 'react')
8+
9+
const versions = fs
10+
.readdirSync(reactDir, { withFileTypes: true })
11+
.filter((entry) => entry.isDirectory())
12+
.filter((entry) =>
13+
fs.existsSync(path.join(reactDir, entry.name, 'package.json')),
14+
)
15+
.map((entry) => entry.name)
16+
.sort()
17+
18+
for (const version of versions) {
19+
const dir = path.join(reactDir, version)
20+
21+
console.log(`Starting React ${version} tests`)
22+
23+
execSync('npm i --no-package-lock --quiet --no-progress', {
24+
cwd: dir,
25+
stdio: 'inherit',
26+
})
27+
28+
try {
29+
execSync(
30+
`REACT_VERSION=${version} npx jest --config ./scripts/jest/config.src.js --coverage false`,
31+
{ stdio: 'inherit' },
32+
)
33+
} catch {
34+
console.error(`Fail testing React ${version}`)
35+
process.exit(1)
36+
}
37+
38+
console.log(`Success testing React ${version}`)
39+
}

src/createTimeout.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,18 @@
1+
// Uses requestAnimationFrame rather than setTimeout for smoother animation
2+
// timing. Note that rAF is throttled or paused in background tabs, so progress
3+
// will stall when the tab is hidden and resume when it regains focus.
14
export const createTimeout = () => {
25
let handle: number | undefined
36

47
const cancel = (): void => {
5-
if (handle) {
8+
if (handle !== undefined) {
69
window.cancelAnimationFrame(handle)
710
}
811
}
912

1013
const schedule = (callback: () => void, delay: number): void => {
14+
cancel()
15+
1116
let deltaTime
1217
let start: number | undefined
1318
const frame: FrameRequestCallback = (time) => {

0 commit comments

Comments
 (0)