-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: algorithm for determine the end of build in watch mode #438
Conversation
} | ||
function end() { | ||
watcherCounter++ | ||
if (watcherCounter === result.length) { | ||
watcherCounter-- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this have concurrence issue? Last time I checked the number it's shorter than the actual build time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How to reproduce the concurrence issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can log all the build duration and compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Take bunchee as an example, when I run the command pnpm bunchee -w
in the project, it will start 4 jobs, watcherCounter will increase to 4 and then decrease to 0 one by one. After I modify bundle.js
and saved, it will restart 2 jobs, watcherCounter will increase to 2 and then decrease to 0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was testing against multi-entries integration test, it has more jobs spinning up together. Is that one also logging the same data for you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried in multi-entries test, still feel good.
Here are steps:
- open bunchee project, execute
pnpm build
,pnpm install
,npm link
in turn. - cd
test/integration/multi-entries
, executebunchee -w
- modify some files and saved.
Here is a sample:
bunchee -w
Watching project /Users/nnecec/Github/bunchee/test/integration/multi-entries...
0
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
21
20
19
18
17
16
15
14
13
12
11
10
9
8
7
6
5
4
3
2
1
0
✓ Build in 7681.08ms
0
1
1
0
✓ Build in 1155.92ms
0
1
1
0
✓ Build in 761.12ms
0
1
2
3
4
4
3
2
1
0
✓ Build in 2460.70ms
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good, let's ship it, thanks!
if we print
console.log(startTime, watcherCount)
inend
function, it looks like:❯ bunchee -w Watching project /Users/nnecec/Github/bunchee... 523.054045021534 1 523.054045021534 2 523.054045021534 3 523.054045021534 4 ✓ Build in 3250.67ms 523.054045021534 5 523.054045021534 6 523.054045021534 7 523.054045021534 8 523.054045021534 9 523.054045021534 10
logWatcherBuildTime
will be executed only once at the beginning. So we need resetstartTime
andwatcherCount
in thestart
function.Also, if we modify a file, it may not trigger a rebuild of all the entry files, so the
watcherCount
maybe not equal toresult.length
.