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

Rename schedule to scheduler #13683

Merged
merged 1 commit into from Sep 19, 2018

Conversation

Projects
None yet
8 participants
@gaearon
Member

gaearon commented Sep 19, 2018

Ran the fixtures and I think everything works.

@@ -38,12 +38,12 @@ if (__UMD__) {
// This re-export is only required for UMD bundles;
// CJS bundles use the shared NPM package.
Object.assign(ReactSharedInternals, {
Schedule: {
Scheduler: {

This comment has been minimized.

@gaearon

gaearon Sep 19, 2018

Member

I think this means a newer React wouldn't work with an older ReactDOM, or vice versa. Since it's scoped to UMDs only I think that's fine and I wouldn't worry about going to extra lengths to make this work for mismatching versions.

@gaearon gaearon requested review from bvaughn and acdlite Sep 19, 2018

@sizebot

This comment has been minimized.

sizebot commented Sep 19, 2018

Details of bundled changes.

Comparing: 9b70816...f7973b1

scheduler

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
scheduler.development.js n/a n/a 0 B 19.17 KB 0 B 5.74 KB UMD_DEV
scheduler.production.min.js n/a n/a 0 B 3.16 KB 0 B 1.53 KB UMD_PROD
scheduler.development.js n/a n/a 0 B 13.84 KB 0 B 4.24 KB NODE_DEV
scheduler.production.min.js n/a n/a 0 B 3.18 KB 0 B 1.39 KB NODE_PROD
Scheduler-dev.js n/a n/a 0 B 14.02 KB 0 B 4.27 KB FB_WWW_DEV
Scheduler-prod.js n/a n/a 0 B 8.13 KB 0 B 2.06 KB FB_WWW_PROD
scheduler-tracking.development.js n/a n/a 0 B 10.27 KB 0 B 2.35 KB NODE_DEV
scheduler-tracking.production.min.js n/a n/a 0 B 719 B 0 B 374 B NODE_PROD
scheduler-tracking.profiling.min.js n/a n/a 0 B 3.26 KB 0 B 992 B NODE_PROFILING
SchedulerTracking-dev.js n/a n/a 0 B 10.36 KB 0 B 2.28 KB FB_WWW_DEV
SchedulerTracking-prod.js n/a n/a 0 B 899 B 0 B 425 B FB_WWW_PROD
SchedulerTracking-profiling.js n/a n/a 0 B 6.82 KB 0 B 1.23 KB FB_WWW_PROFILING
scheduler-tracing.development.js n/a n/a 0 B 10.25 KB 0 B 2.35 KB NODE_DEV
scheduler-tracing.production.min.js n/a n/a 0 B 719 B 0 B 374 B NODE_PROD
scheduler-tracing.profiling.min.js n/a n/a 0 B 3.26 KB 0 B 991 B NODE_PROFILING
SchedulerTracing-dev.js n/a n/a 0 B 10.34 KB 0 B 2.27 KB FB_WWW_DEV
SchedulerTracing-prod.js n/a n/a 0 B 899 B 0 B 425 B FB_WWW_PROD
SchedulerTracing-profiling.js n/a n/a 0 B 6.82 KB 0 B 1.23 KB FB_WWW_PROFILING

Generated by 🚫 dangerJS

@gaearon gaearon merged commit 7ea3ca1 into facebook:master Sep 19, 2018

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

@gaearon gaearon deleted the gaearon:rename-s branch Sep 19, 2018

@NE-SmallTown

This comment has been minimized.

Contributor

NE-SmallTown commented Sep 19, 2018

Can I ask why we use 'schedule' rather than 'scheduler' when we rename it before? IIRC, we use 'react-scheduler' before but rename it to 'schedule', sorry about don‘t come up with this confusion in that PR

@bvaughn

This comment has been minimized.

Contributor

bvaughn commented Sep 19, 2018

We didn't own the scheduler NPM package so we couldn't use it. It was the preferred name originally but we settled for schedule because that was all we had at the time.

@NMinhNguyen

This comment has been minimized.

Contributor

NMinhNguyen commented Sep 19, 2018

Perhaps a silly question but couldn't easily find answers to this, but has @react/scheduler been considered?

@bvaughn

This comment has been minimized.

Contributor

bvaughn commented Sep 19, 2018

We have talked a little about potentially using @react/<package> for some future packages– but schedule/scheduler isn't inherently specific to React. (That's why we didn't just stick with our original name react-scheduler.)

@gaearon gaearon referenced this pull request Oct 23, 2018

Merged

Add 16.6.0 changelog #13927

@monkindey

This comment has been minimized.

Contributor

monkindey commented Oct 24, 2018

I think it will break the webpack config when we use the react-dom/profiling.

schedule ==> scheduler

Before

module.exports = {
  //...
  resolve: {
    alias: {
      'react-dom': 'react-dom/profiling',
      'schedule/tracing': 'schedule/tracing-profiling',
    }
  }
};

After

module.exports = {
  //...
  resolve: {
    alias: {
      'react-dom': 'react-dom/profiling',
      'scheduler/tracing': 'scheduler/tracing-profiling',
    }
  }
};
@gaearon

This comment has been minimized.

Member

gaearon commented Oct 24, 2018

Yeah, we need to change these instructions.

@monkindey

This comment has been minimized.

Contributor

monkindey commented Oct 24, 2018

Here ?

@bvaughn

This comment has been minimized.

Contributor

bvaughn commented Oct 24, 2018

Both fb.me/react-profiling and fb.me/react-interaction-tracing have been udpated!

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