Rename "react-scheduler" package to "schedule" #13543
Conversation
| "scripts": { | ||
| "build": "rm -f output.js && browserify ./input.js -o output.js" | ||
| "build": "rm -f output.js && browserify ./input.js -o output.js", | ||
| "prebuild": "rm -rf ./node_modules/react-dom/node_modules/schedule" |
There was a problem hiding this comment.
Sketchy. :/ If we can't figure out how to make Yarn work with this, let's just remove dependencies completely, and then make this cp -r ../../../../build/node_modules/* ./node_modules/?
Details of bundled changes.Comparing: 8a1e396...f2132a7 schedule
Generated by 🚫 dangerJS |
|
That's interesting. CI is failing on |
| "loose-envify": "^1.1.0", | ||
| "object-assign": "^4.1.1", | ||
| "prop-types": "^15.6.2", | ||
| "react-scheduler": "^0.1.0-alpha-1" |
There was a problem hiding this comment.
Just wanna confirm this was intentional.
There was a problem hiding this comment.
Yeah, it was– but I'm not sure it was the right decision. Still poking around locally. May revert this.
There was a problem hiding this comment.
If we don't reference it in CJS then it makes sense to me.
There was a problem hiding this comment.
I thought so too, but it results in the scheduler methods (e.g. cancelScheduledWork) being embedded in the package, so... I re-added it.
There was a problem hiding this comment.
If it embeds scheduler methods it means it always imports them regardless. So this is bad by itself (we don't expect them to be in CJS builds). Are they not getting tree shaken due to init time side effects? This might mean our approach with __UMD__ is not enough and we actually need to fork that file.
There was a problem hiding this comment.
The approach seems to be working fine. At least, the bundles look good to me. Removing this dep causes troubles though.
There may be a reasonable explanation. I'm not going to dig into it today though. (Holiday.)
I think this PR is good at this point. The deposit was already there so it's at least not new from this PR.
packages/schedule/package.json
Outdated
| "version": "0.1.0-alpha-1", | ||
| "name": "schedule", | ||
| "version": "0.2.0", | ||
| "private": true, |
* Git moved packages/react-scheduler -> packages/schedule * Global find+replace 'react-scheduler' -> 'schedule' * Global find+replace 'ReactScheduler' -> 'Scheduler' * Renamed remaining files "ReactScheduler" -> "Schedule" * Add thank-you note to schedule package README * Replaced schedule package versions 0.1.0-alpha-1 -> 0.2.0 * Patched our local fixtures to work around Yarn install issue * Removed some fixture hacks
* Git moved packages/react-scheduler -> packages/schedule * Global find+replace 'react-scheduler' -> 'schedule' * Global find+replace 'ReactScheduler' -> 'Scheduler' * Renamed remaining files "ReactScheduler" -> "Schedule" * Add thank-you note to schedule package README * Replaced schedule package versions 0.1.0-alpha-1 -> 0.2.0 * Patched our local fixtures to work around Yarn install issue * Removed some fixture hacks
4 participants
This is mostly all rote search-and-replace.
The only commit that seems sketchy is the last one– c84b3d6. I couldn't get our packages or suspense fixtures to install correctly. Yarn seemed to want to pull the
scheduledependency from NPM even though I'd told it to install the local version. This resulted in two packages being installed: one innode_modulesand one innode_modules/react-dom/node_modules. So I added a pre script to remove the redundant one.We can also test our fixtures without this hack by publishing
scheduleversion 0.2.0 to NPM (which will also unblock the React Native sync).I'm going to step away for a bit (holiday in the US) but I'll check back later... 😄