ReactBinding Rewrite#7
Open
gaymeowing wants to merge 291 commits into
Open
Conversation
added 30 commits
August 11, 2021 09:33
setup CI benchmark script setup benchmarks on CI
LUAFDN-461 Fixes an issue where forwardRef objects did not allow fields to be assigned to them, which will break certain use cases in the wild.
Closes LUAFDN-477. This won't be as thorough with warning as before, but it should cut out a lot of noise that makes it hard to see other, more important dev warnings.
Increments version for a rotriever release
push gh-pages only on master add benchmarks to docs site benchmarks and docs deploy in single workflow
Calculate benchmarks data for each push on PRs. Compares benchmark results to master (instead of previous commit), and avoids retriggering lifecycles while still allowing checks to run and pass.
Update benchmark data.json to comply with the newest benchmarks
* Fix new true positive analysis warning. Submitted a similar PR upstream. * Eliminate unused dependencies to get rid of some 0% coverage in rotriever proxy files.
* Pass key to createChild, updateSlot, add test * Add fix for nested children arrays * Add benchmarks results for b018a4374ff037832d8817c5eecd9a866f937469 skip-push-benchmarks: true * Add benchmarks results for cf67e67847416a3efaf7ad1c48a198e9b322c149 skip-push-benchmarks: true * Add key to single element when changing from array * Add benchmarks results for 7b035d84334cbad245a57eb84dbc880f8fb5e9bb skip-push-benchmarks: true * Add deviation comments to tests * Add benchmarks results for 323aa7586104f0cb31715e4aa488c9e7d9b55c96 skip-push-benchmarks: true * Update Deviations.md * Add benchmarks results for 84a2b81f277f500076c83ad63d365e098a3d9c4d skip-push-benchmarks: true * Don't use expensive isArray, instead directly check if element before passing it newIdx * Add benchmarks results for d77ddcae5064aad786dd7a16a745cf16d8b3a481 skip-push-benchmarks: true
* LUAFDN-540 Limit pcalls in recursivelyCommitLayoutEffects * Add benchmarks results for cfaf6a261dbfcdf1302576111759edfdca95e9d2 skip-push-benchmarks: true * Lowered threshold to 180, added test to break things * Update dev mode and max run depth * Add benchmarks results for 9b17402a805c9c90daa6561a8529599ac138d139 skip-push-benchmarks: true * Fix whitespace issues in test * Add benchmarks results for 55488503ce63674a1a9158484b5a4b3073f7e15d skip-push-benchmarks: true
* Store should re-assign its event-handling methods in its constructor so that they can be called in isolation without reference to self * More minor fixes, mainly removing Map.new * Fix another round of require paths. Workspace is the local package's src directory. Packages is the local package's explicit dependencies as specified in the rotriver.toml. * Fix some warnings relating to keys not being found on the reconciler import. Reconciler is special, in that it returns a function that needs to be called with a HostConfig object. This is our runtime emulation of a bundle/compile-time injection that happens upstream. Talking with Paul, we need to make it so we can import data-only exports like ReactTypeOfMode, without the need for a host config. We'll figure it out next week, but we should be able to run store-test and some of the other self-contained unit tests in the meantime. * Expose ReactFiberFlags in ReactFiberReconciler * Renderer calls aren't instance calls * Introduce Agent type * Properly return exports for cache, even thought the module isn't used. Manually fix another stylua misformat bug. * First pass at a public interface for devtools-shared. Like the shared/ directory upstream, there's no explicit interface (yet).
…sages (#192) * Make ReactFiberComponentStack exception handling durable against all kinds of error shapes that could be thrown * export unstable_batchedUpdates to be consistent with other renderers. * export createTextInstance and throw an unimplemented error for clearer feedback when porting upstream tests. * Fix some of the string formatting issues lying in wait mentioned in the JIRA ticket. This functionality is already covered upstream by a test file we haven't ported yet, so port the relevant portions of that file. * Enable many newly ported upstream tests. * Review all console.error messages for bugs and issues. Found a number of issues and fixed them. Most of the bugs would have been found with the upstream createReactClassIntegration-test file, which we haven't ported yet. Saving that step for another ticket. * Review all the invariant messages, and fix a few issues found. Port an upstream test file over that would have covered things. Add some deviation and TODO comments. * Add missing upstream comments, remove some out-of-date deciation/FIXME comments. * Fix https links * Fix a very interesting bug that would have bit us deeply. This was partially due to the deviated logic around when we can check for 'ref.current', and that we weren't consistent about using hard errors around use of string refs.
* I could not figure out how to do the console spying, so I created a completely deviant `toLogDev` matcher. We should do a proper `spyOnDevAndProd` implementation in jest, which will likely need some intimacy with the `console` in LuauPolyfill. * The Suspense tests for DebugTracing aren't passing because we never get the `resolve` message. Files LUAFDN-564 to track that future work.
* change scheduler's work window
* Switch back to setTimeout skip-push-benchmarks: true
* Fix numerous wrong-code bugs in the initial react-devtools-shared and react-debug-tools translation
* Align upstream types and comments in react-devtools-shared and react-debug-tools. Fix all nonstrict and most strict analysis issues. Add nonstrict to robloxrc for both packages.
* Fix slightly misaligned types and some missing types (`any`) in react-reconciler and shared
* Move the `Dispatcher` type to Shared so it can be used without running into circular dependencies
* We actually get 'code is too complex to typecheck' warnings when adding `Dispatcher` type annotation in a key place. Add a FIXME comments and make it `Object` for now.
* Today I learned that a function returning void ('()') will not match a function type returning 'any'. It *will* match '...any', though.
* Format react-devtools-shared and react-debug-tools using StyLua. Update StyLua to run on these entire directories.
There are two steps we need to take: Switch to using task.delay Separate yield tracking from frame boundary tracking; decide dynamically when to delay vs. simply yielding This PR will track only step 1. I've created LUAFDN-603 for the second part.
* Set run depth checks to use MAX_RUN_DEPTH * update associated test * update deviation text * Add benchmarks results for d5070ccd96a0422a5c60708c36df78c9e94fb6a9 skip-push-benchmarks: true * Add benchmarks results for 7f52b3f9e49cecbbd101ce43a31c3cd3c5157e60 skip-push-benchmarks: true
This addresses the issue that folks hit with jest-roblox, where robloxdev-cli convert has different type analysis behavior than roblox-cli analyze
Moves a warning that was happening in non-dev to only happen in dev mode.
* Update to rc.3
Fix bug where reconcileChildrenArray does not pass a value for tableKey causing instance names to be incorrect
… (#207) * Align types that would have allowed Luau type analysis to surface this bug. * Found several more hard runtime error bugs in string formatting by looking closely at strict mode. Another example of upstream using a field that doesn't exist on the type and is never initialized. Marked with a TODO. * Avoid dereferecing a function with a field. We don't currently use the value anyway, so comment out offending code. Comment out now-unused code. Filed a ticket to pick up this color-coding work. * Move UpdateQueue<> types to ReactInternalTypes to avoid future initialization order issues.
* Made types compatible with Luau RFC recursive-type-restriction
… bugs (#206) * Fix hard runtime errors caused by multiple mistranslation of string concatenation. Last bug was some JS WAT behavior upstream relied on. Two previously skipped tests now pass.
…rror (#205) * Fix a mistranslation bug, and fortify the types that *should* find it once Luau issues are fixed. Luau issue filed as CLI-46597 * Put react-cache under StyLua enforcement
* Type the hooks exports. Align more types with upstream, which fixes quite a few strict mode analysis errors where it couldn't infer types. Fix other places where strict analysis couldn't deduce types. skip-push-benchmarks: true
* Switch promise dependency to internal one skip-push-benchmarks: true
Ports ReactFreshRuntime.lua and the remainder of ReactFiberHotReloading.new.lua to support HMR. ---------
Closes [UIBLOX-2787](https://roblox.atlassian.net/browse/UIBLOX-2787). Implements a small example app to make testing DevTools and other React features easier. The example app itself is taken from the HMR repo.
### Why are the changes being made? Closes [UIBLOX-2778](https://roblox.atlassian.net/browse/UIBLOX-2778). Fixes a few bugs with native element highlights in Devtools: - We now make sure that an instance is a GuiBase2d before trying to read UI properties - OverlayTip now accounts for a global ZIndex - Focusing the viewport no longer ends selection (e.g. clicking from devtools into studio) - Overlapping highlight rects no longer stack their transparency
… (#461) ### Why are the changes being made? Closes [UIBLOX-2757](https://roblox.atlassian.net/browse/UIBLOX-2757). Adds check for touch events when doing element selection in devtools. This adds minimal support for the mobile emulator, but the UX still isn't ideal since you have to hold your pointer down to preview the selection. Unfortunately, I don't think there's a way around this since input events only fire in the emulator when the pointer is down. This shouldn't merge until after #460 because I need to apply the new check to the other input event that PR introduces. ### What changes are being made? - Adds `isInputValid` utility to React DevTools Highlighter to support touch input for Roblox Studio's mobile emulator and updates input handling accordingly. --- - [x] AI-assisted summary reviewed and verified by author
### Why are the changes being made? Closes [UIBLOX-2770](https://roblox.atlassian.net/browse/UIBLOX-2770). ### What changes are being made? - Fixes root name display in React DevTools by initializing flags before importing `ReactDevtoolsCore`, replacing `mountedRoots` table operations with `Set` methods, and using fiber key for root name resolution. --- - [x] AI-assisted summary reviewed and verified by author ---------
### Why are the changes being made? Release version 17.3.2 ### What changes are being made? - Updates version in `rotriever.toml` from `17.3.1` to `17.3.2` for new release preparation --- - [x] AI-assisted summary reviewed and verified by author
### Why are the changes being made? Closes [UIBLOX-1997](https://roblox.atlassian.net/browse/UIBLOX-1997). *Put your pull request body here!* Checklist before submitting: * [ ] Added/updated relevant tests * [ ] Added/updated documentation ### What changes are being made? - Introduces `ReactGlobals` package to replace `_G` for React's global variables, ensuring scoped and type-safe global state per React instance. - Refactors all modules and tests to use `ReactGlobals`, updates dependencies to include the new package, and maintains backward compatibility. - Adds documentation for `ReactGlobals` and resolves cross-plugin state leakage issue described in UIBLOX-1997. --- - [ ] AI-assisted summary reviewed and verified by author
Closes [UIBLOX-2900](https://roblox.atlassian.net/browse/UIBLOX-2900). Adds a `profileOnStart` option when connecting to DevTools. This makes the agent start profiling as soon as the DevTools hook is setup and initialized, before any tree is even mounted. This is a deviation from upstream, which doesn't expose a public API for starting the profiler immediately.
### Why are the changes being made? Add context about changes here. ### What changes are being made? - Updates `roact-alignment` to version `17.3.3` by modifying `ReactVersion.lua` and `rotriever.toml`. --- - [ ] AI-assisted summary reviewed and verified by author
### Why are the changes being made? Closes [UIBLOX-2937](https://roblox.atlassian.net/browse/UIBLOX-2937). Implements a new React Telemetry package for internal-only telemetry collection. This is currently only used for DevTools, but is designed to be easy to use in other applications later. ### What changes are being made? - Adds new `react-telemetry` module for internal telemetry reporting in React apps, including event logging for DevTools connections via `TelemetryService`. - Introduces feature flag `ReactTelemetryEnabled` to gate telemetry functionality and supports custom fields for event data. - Adds configuration and utility files (`.luaurc`, `.robloxrc`, `rotriever.toml`, README, `reportCounter`, `getCustomFields`) to structure and document the module. --- - [x] AI-assisted summary reviewed and verified by author
### Why are the changes being made? Closes [UIBLOX-3019](https://roblox.atlassian.net/browse/UIBLOX-3019). Ports the minimal amount of code from `react-devtools-timeline` required by the latest version of `react-devtools-shared`. This is just Luau types and a couple of constants, but could expand in the future. I considered just inlining these in devtools shared, but wanted to stay aligned with upstream. edit: I also apprantely updated styling for some example files. probably wanna keep that? ### What changes are being made? - Ports minimal subset of `react-devtools-timeline` to support latest `react-devtools-shared` by adding `constants.lua` and `types.lua` with upstream constants and types - Adds new module configuration files `.luaurc`, `.robloxrc`, `default.project.json`, and `rotriever.toml` for `react-devtools-timeline` - Updates example files for compatibility and formatting consistent with Roblox Luau environment --- - [ ] AI-assisted summary reviewed and verified by author
* nolonger create a connection table for each connection to a binding * use a boolean for indicating if a callback is suspended * only use one callbacks table, instead of one for suspended callbacks and one for all callbacks
Author
Ran on: 
Benchmark place: ReactBindingRewriteBenchmark.zip |
* make naming of bindings consistent * improve error messages
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rewrites ReactBinding to be cleaner, and also use less memory as its now properly using metatables.
__subscribeToBindingas depricated as every binding now has a:_subscribe()method that should be used instead_:subscribe()updatefunction not iterate twice when firing off callbacksChecklist before submitting: