Skip to content
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

[Bugfix] Pending state is always user-blocking #17382

Merged
merged 1 commit into from Nov 15, 2019

Conversation

acdlite
Copy link
Member

@acdlite acdlite commented Nov 15, 2019

Fixes a bug where isPending is only set to true if startTransition is called from inside an input event. That's usually the case, but not always.

Now it works regardless of where you call it.

Fixes #17272

Fixes a bug where `isPending` is only set to `true` if `startTransition`
is called from inside an input event. That's usually the case, but
not always.

Now it works regardless of where you call it.
@codesandbox-ci
Copy link

@codesandbox-ci codesandbox-ci bot commented Nov 15, 2019

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit c292c83:

Sandbox Source
pedantic-fog-l8525 Configuration

@gaearon
Copy link
Member

@gaearon gaearon commented Nov 15, 2019

Btw we track this in #17272

@acdlite acdlite merged commit 2586303 into facebook:master Nov 15, 2019
1 of 2 checks passed
},
[value, config],
);
return prevValue;
}

function startTransition(setPending, config, callback) {
const priorityLevel = getCurrentPriorityLevel();
runWithPriority(
Copy link
Collaborator

@sebmarkbage sebmarkbage Nov 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like we do this in a couple of internal places which seems like a very heavy weight way to do this. Going through the scheduler to do this. This adds to my suspicion that reading priorities from scheduler is the wrong model. It's not that scheduler dictates the priorities but it sometimes can be used as a default.

Copy link
Member Author

@acdlite acdlite Nov 17, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, it's on my list of things to consider during expiration times refactor. Didn't want to couple it to this bugfix.

},
);
runWithPriority(
priorityLevel > NormalPriority ? NormalPriority : priorityLevel,
Copy link
Collaborator

@sebmarkbage sebmarkbage Nov 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly here, it's not obvious to me that calling startTransition should change the running "current" queue of the scheduler. It should only change the priority inside React. They're not necessarily 1:1.

let useTransition;
let act;

describe('ReactHooksWithNoopRenderer', () => {
Copy link
Member

@gaearon gaearon Nov 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy paste name

@sizebot
Copy link

@sizebot sizebot commented Nov 16, 2019

Details of bundled changes.

Comparing: 8e74a31...c292c83

react-native-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactNativeRenderer-dev.js -0.0% 0.0% 749.78 KB 749.51 KB 158.66 KB 158.73 KB RN_FB_DEV
ReactNativeRenderer-prod.js -0.2% 🔺+0.1% 277.75 KB 277.31 KB 47.49 KB 47.54 KB RN_FB_PROD
ReactNativeRenderer-profiling.js -0.2% +0.1% 286.7 KB 286.26 KB 49.2 KB 49.26 KB RN_FB_PROFILING
ReactFabric-dev.js -0.0% 0.0% 755.18 KB 754.93 KB 159.49 KB 159.55 KB RN_FB_DEV
ReactFabric-prod.js -0.2% 🔺+0.1% 269.4 KB 268.96 KB 46.05 KB 46.1 KB RN_FB_PROD
ReactFabric-profiling.js -0.2% +0.1% 279.53 KB 279.1 KB 47.92 KB 47.98 KB RN_FB_PROFILING
ReactNativeRenderer-dev.js -0.0% 0.0% 749.61 KB 749.34 KB 158.58 KB 158.64 KB RN_OSS_DEV
ReactNativeRenderer-prod.js -0.2% 🔺+0.1% 277.36 KB 276.92 KB 47.42 KB 47.46 KB RN_OSS_PROD
ReactNativeRenderer-profiling.js -0.2% +0.1% 286.32 KB 285.88 KB 49.13 KB 49.19 KB RN_OSS_PROFILING
ReactFabric-dev.js -0.0% 0.0% 755 KB 754.75 KB 159.41 KB 159.47 KB RN_OSS_DEV
ReactFabric-prod.js -0.2% 🔺+0.1% 269.05 KB 268.62 KB 45.97 KB 46.03 KB RN_OSS_PROD
ReactFabric-profiling.js -0.2% +0.1% 279.19 KB 278.75 KB 47.85 KB 47.91 KB RN_OSS_PROFILING

Size changes (stable)

Generated by 🚫 dangerJS against c292c83

@sizebot
Copy link

@sizebot sizebot commented Nov 16, 2019

Details of bundled changes.

Comparing: 8e74a31...c292c83

react-native-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactFabric-prod.js -0.2% 🔺+0.1% 269.41 KB 268.98 KB 46.06 KB 46.11 KB RN_FB_PROD
ReactNativeRenderer-dev.js -0.0% 0.0% 749.62 KB 749.35 KB 158.59 KB 158.65 KB RN_OSS_DEV
ReactFabric-profiling.js -0.2% +0.1% 279.54 KB 279.11 KB 47.93 KB 47.99 KB RN_FB_PROFILING
ReactNativeRenderer-prod.js -0.2% 🔺+0.1% 277.76 KB 277.32 KB 47.5 KB 47.55 KB RN_FB_PROD
ReactNativeRenderer-profiling.js -0.2% +0.1% 286.71 KB 286.28 KB 49.21 KB 49.27 KB RN_FB_PROFILING
ReactFabric-dev.js -0.0% 0.0% 755.01 KB 754.76 KB 159.41 KB 159.48 KB RN_OSS_DEV
ReactFabric-prod.js -0.2% 🔺+0.1% 269.06 KB 268.63 KB 45.98 KB 46.04 KB RN_OSS_PROD
ReactFabric-profiling.js -0.2% +0.1% 279.2 KB 278.76 KB 47.86 KB 47.92 KB RN_OSS_PROFILING
ReactFabric-dev.js -0.0% 0.0% 755.19 KB 754.95 KB 159.5 KB 159.56 KB RN_FB_DEV
ReactNativeRenderer-prod.js -0.2% 🔺+0.1% 277.37 KB 276.93 KB 47.43 KB 47.47 KB RN_OSS_PROD
ReactNativeRenderer-profiling.js -0.2% +0.1% 286.33 KB 285.89 KB 49.14 KB 49.2 KB RN_OSS_PROFILING
ReactNativeRenderer-dev.js -0.0% 0.0% 749.79 KB 749.53 KB 158.67 KB 158.73 KB RN_FB_DEV

react-art

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactART-dev.js -0.0% +0.1% 617.68 KB 617.41 KB 128.37 KB 128.43 KB FB_WWW_DEV
ReactART-prod.js -0.2% 🔺+0.1% 236.28 KB 235.84 KB 39.7 KB 39.76 KB FB_WWW_PROD
react-art.development.js -0.0% 0.0% 673.56 KB 673.38 KB 146.06 KB 146.12 KB UMD_DEV
react-art.production.min.js -0.1% 🔺+0.2% 106.75 KB 106.67 KB 32.36 KB 32.42 KB UMD_PROD
react-art.development.js -0.0% 0.0% 604.23 KB 604.01 KB 128.68 KB 128.73 KB NODE_DEV
react-art.production.min.js -0.2% 0.0% 71.75 KB 71.64 KB 21.48 KB 21.48 KB NODE_PROD

react-dom

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-dom.profiling.min.js -0.1% +0.1% 123.64 KB 123.53 KB 38.81 KB 38.84 KB NODE_PROFILING
react-dom-server.browser.development.js 0.0% 0.0% 140.3 KB 140.3 KB 36.85 KB 36.85 KB UMD_DEV
react-dom-server.browser.production.min.js 0.0% 0.0% 20.39 KB 20.39 KB 7.48 KB 7.48 KB UMD_PROD
react-dom-test-utils.development.js 0.0% 0.0% 56.21 KB 56.21 KB 15.55 KB 15.55 KB UMD_DEV
react-dom-test-utils.production.min.js 0.0% 0.0% 11.18 KB 11.18 KB 4.15 KB 4.15 KB UMD_PROD
ReactDOMServer-dev.js 0.0% 0.0% 139.57 KB 139.57 KB 35.4 KB 35.4 KB FB_WWW_DEV
react-dom-test-utils.development.js 0.0% 0.0% 54.48 KB 54.48 KB 15.22 KB 15.22 KB NODE_DEV
ReactDOMServer-prod.js 0.0% 0.0% 48.79 KB 48.79 KB 11.13 KB 11.13 KB FB_WWW_PROD
react-dom.development.js -0.0% 0.0% 953.68 KB 953.5 KB 215.55 KB 215.61 KB UMD_DEV
react-dom.production.min.js -0.1% -0.0% 119.67 KB 119.59 KB 38.42 KB 38.42 KB UMD_PROD
react-dom.profiling.min.js -0.1% +0.1% 123.35 KB 123.27 KB 39.49 KB 39.53 KB UMD_PROFILING
react-dom.development.js -0.0% 0.0% 947.78 KB 947.57 KB 213.98 KB 214.03 KB NODE_DEV
react-dom.production.min.js -0.1% 0.0% 119.84 KB 119.73 KB 37.77 KB 37.79 KB NODE_PROD
react-dom-server.node.production.min.js 0.0% -0.0% 20.72 KB 20.72 KB 7.61 KB 7.61 KB NODE_PROD
react-dom-server.browser.production.min.js 0.0% -0.0% 20.31 KB 20.31 KB 7.46 KB 7.46 KB NODE_PROD
react-dom-unstable-native-dependencies.development.js 0.0% 0.0% 60.14 KB 60.14 KB 15.8 KB 15.8 KB UMD_DEV
react-dom-unstable-native-dependencies.production.min.js 0.0% 0.0% 10.73 KB 10.73 KB 3.67 KB 3.67 KB UMD_PROD
ReactDOM-dev.js -0.0% 0.0% 975.9 KB 975.63 KB 216.5 KB 216.56 KB FB_WWW_DEV
ReactDOM-prod.js -0.1% 🔺+0.1% 402.8 KB 402.36 KB 73.25 KB 73.3 KB FB_WWW_PROD
react-dom-unstable-fizz.node.development.js 0.0% +0.1% 4.42 KB 4.42 KB 1.65 KB 1.65 KB NODE_DEV
ReactDOM-profiling.js -0.1% +0.1% 403.72 KB 403.28 KB 73.79 KB 73.85 KB FB_WWW_PROFILING
react-dom-unstable-native-dependencies.production.min.js 0.0% 0.0% 10.48 KB 10.48 KB 3.57 KB 3.58 KB NODE_PROD

ReactDOM: size: 0.0%, gzip: 0.0%

Size changes (experimental)

Generated by 🚫 dangerJS against c292c83

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants