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

Removed ref-getter methods from "schedule" API #13561

Merged
merged 1 commit into from
Sep 5, 2018

Conversation

bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Sep 5, 2018

First two steps of #13559:

I tested the packaging and tracking fixtures, as well as the "suspense" demo app, and hand-inspected the react, react-dom, and schedule NPM bundles. All seemed in order.

* Removed 'private' ref methods from UMD forwarding API
* Replaced getters with exported constants since they were no longer referenced for UMD forwarding
@bvaughn bvaughn mentioned this pull request Sep 5, 2018
7 tasks
@bvaughn bvaughn changed the title Cleaned up 'schedule' API wrt interactions and subscriber ref: Removed ref-getter methods from "schedule" API Sep 5, 2018
@pull-bot
Copy link

pull-bot commented Sep 5, 2018

React: size: -0.4%, gzip: -0.1%

Details of bundled changes.

Comparing: fb88fd9...3c2a934

react

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react.development.js -0.4% -0.4% 83.77 KB 83.44 KB 22.73 KB 22.63 KB UMD_DEV
react.production.min.js -0.4% -0.1% 9.59 KB 9.55 KB 3.96 KB 3.96 KB UMD_PROD

schedule

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
schedule.development.js n/a n/a 0 B 19.17 KB 0 B 5.74 KB UMD_DEV
schedule.production.min.js n/a n/a 0 B 3.16 KB 0 B 1.53 KB UMD_PROD
schedule-tracking.development.js -2.1% -4.1% 10.83 KB 10.61 KB 2.62 KB 2.52 KB NODE_DEV
schedule-tracking.production.min.js -5.8% -1.3% 765 B 721 B 386 B 381 B NODE_PROD
schedule-tracking.profiling.min.js +12.9% 0.0% 2.89 KB 3.26 KB 997 B 997 B NODE_PROFILING
ScheduleTracking-dev.js -2.1% -5.0% 10.43 KB 10.21 KB 2.35 KB 2.23 KB FB_WWW_DEV
ScheduleTracking-prod.js -5.9% -0.7% 949 B 893 B 428 B 425 B FB_WWW_PROD
ScheduleTracking-profiling.js +0.2% -1.7% 6.8 KB 6.81 KB 1.25 KB 1.23 KB FB_WWW_PROFILING

Generated by 🚫 dangerJS

Copy link
Collaborator

@sebmarkbage sebmarkbage left a comment

Choose a reason for hiding this comment

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

The ref object thing is pretty OCaml/React specific.

Would it make sense to just have one object with two mutable fields on it? One field for interactions and one field for subscribers?

@bvaughn
Copy link
Contributor Author

bvaughn commented Sep 5, 2018

Would it make sense to just have one object with two mutable fields on it? One field for interactions and one field for subscribers?

Sure, that could work too. I don't have a strong preference.

I think we'll probably want to revisit these two properties before a 1.0 release anyway, regarding our conversation yesterday. Sounds like they may be more appropriate to expose as an "advanced API" rather than "private".

I guess I could move them behind an advanced wrapper? I just don't know what to call the thing.

Let's make that change in a future commit. 😄

@bvaughn bvaughn merged commit 9a110eb into facebook:master Sep 5, 2018
@bvaughn bvaughn deleted the issues/13559 branch September 5, 2018 14:29
jetoneza pushed a commit to jetoneza/react that referenced this pull request Jan 23, 2019
…ook#13561)

* Removed 'private' ref methods from UMD forwarding API
* Replaced getters with exported constants since they were no longer referenced for UMD forwarding
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.

4 participants