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

[size: tiny] Rename methods exposed by ReactScheduler #12742

Closed

Conversation

flarnie
Copy link
Contributor

@flarnie flarnie commented May 4, 2018

Going to make incremental PRs with the changes we discussed. This is a quick and easy one to start with.

what is the change?:
rIC -> scheduleSerialCallback
We will later expose a second method called scheduleDeferredCallback,
for different priority levels.

cIC -> cancelScheduledCallback
This method can be used to cancel callbacks scheduled at either serial
or deferred priority.

why make this change?:
Originally this module contained a polyfill for requestIdleCallback
and cancelIdleCallback but we are changing the behavior so it's no
longer just a polyfill. The new names are more semantic and distinguish
this from the original polyfill functionality.

test plan:
Ran the tests

**what is the change?:**
`rIC` -> `scheduleSerialCallback`
We will later expose a second method called `scheduleDeferredCallback`,
for different priority levels.

`cIC` -> `cancelScheduledCallback`
This method can be used to cancel callbacks scheduled at either serial
or deferred priority.

**why make this change?:**
Originally this module contained a polyfill for `requestIdleCallback`
and `cancelIdleCallback` but we are changing the behavior so it's no
longer just a polyfill. The new names are more semantic and distinguish
this from the original polyfill functionality.

**test plan:**
Ran the tests
@bvaughn
Copy link
Contributor

bvaughn commented May 4, 2018

if rIC -> scheduleSerialCallback, then shouldn't cIC -> cancelSerialCallback ?

@gaearon
Copy link
Collaborator

gaearon commented May 4, 2018

I don’t think so.

We will later expose a second method called scheduleDeferredCallback,
for different priority levels.

This method can be used to cancel callbacks scheduled at either serial
or deferred priority
.

(Emphasis mine)

@bvaughn
Copy link
Contributor

bvaughn commented May 4, 2018

Ah, my bad. I skimmed the PR description and missed that.

Seems a little odd, initially, to have separate scheduling methods and a shared cancel method, but I haven't thought about it much. 😄

@flarnie
Copy link
Contributor Author

flarnie commented May 4, 2018

Yea, I had initially assumed we would have a separate cancel method for each priority, but we don't actually need different behavior for cancelling. It's always the same.

@acdlite
Copy link
Collaborator

acdlite commented May 4, 2018

Let's bikeshed these offiline. I don't like scheduleSerialCallback because it's the event that's serial, not the callback itself. And I don't like scheduleDeferredCallback because part of the reason for creating this module is to blunt the negative connotation of words like "async" and "idle" and "deferred." We've observed product engineers are scared to wrap anything in these APIs, even stuff like network events, because they connote slowness.

@flarnie flarnie closed this May 4, 2018
@flarnie
Copy link
Contributor Author

flarnie commented May 4, 2018

@sebmarkbage had brought up that we haven't renamed these methods yet - so let's just bookmark that we're holding off on choosing names until we have time to discuss at length.

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

Successfully merging this pull request may close these issues.

None yet

5 participants