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

[activity] rename unstable_Offscreen to unstable_Activity #27640

Merged
merged 3 commits into from Nov 2, 2023

Conversation

kassens
Copy link
Member

@kassens kassens commented Nov 2, 2023

Activity is the current candidate name. This PR starts the rename work by renaming the exported unstable component name.

NOTE: downstream consumers need to rename the import when updating to this commit.

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Nov 2, 2023
@react-sizebot
Copy link

react-sizebot commented Nov 2, 2023

Comparing: a17467e...9f6a8bf

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 175.24 kB 175.24 kB = 54.53 kB 54.53 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 177.37 kB 177.37 kB = 55.19 kB 55.19 kB
facebook-www/ReactDOM-prod.classic.js = 567.70 kB 567.70 kB = 99.96 kB 99.96 kB
facebook-www/ReactDOM-prod.modern.js = 551.56 kB 551.56 kB = 97.06 kB 97.06 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 9f6a8bf

@noahlemen
Copy link
Member

downstream consumers need to rename the import when updating to this commit

would it make sense to leave the unstable_Offscreen export in place alongside unstable_Activity temporarily until this is fully cleaned up?

@kassens kassens merged commit ce2bc58 into facebook:main Nov 2, 2023
36 checks passed
@kassens kassens deleted the activity branch November 2, 2023 20:13
github-actions bot pushed a commit that referenced this pull request Nov 2, 2023
`Activity` is the current candidate name. This PR starts the rename work
by renaming the exported unstable component name.

NOTE: downstream consumers need to rename the import when updating to
this commit.

DiffTrain build for [ce2bc58](ce2bc58)
@markerikson
Copy link
Contributor

I'm curious, what's the rationale / intent behind the name change?

@bvaughn
Copy link
Contributor

bvaughn commented Nov 4, 2023

This is an unexpected name change. Any link to discussion around the motivations for this rename? Didn't see anything in the RFC repo.

@sebmarkbage
Copy link
Collaborator

An unreleased feature could change name many times more before release. We don’t use the RFC repo as the process anymore. It should probably indicate as such.

@bvaughn
Copy link
Contributor

bvaughn commented Nov 4, 2023

I wasn’t saying the name change was a problem, just that it was unexpected. “Activity” seems like an odd choice so I’d be interested in learning more about the reasoning that went into it.

@petetnt
Copy link
Contributor

petetnt commented Nov 7, 2023

We don’t use the RFC repo as the process anymore. It should probably indicate as such.

At all or for this particular feature?

@gaearon
Copy link
Collaborator

gaearon commented Nov 8, 2023

Seeing this being linked to so I want to answer the questions. (Apparently I can't write short answers; sorry!)

In reactjs/rfcs#200, we reflected the reality at the time: we primarily used RFCs as a channel to announce practically finished APIs that have received sufficient vetting that we're very confident in them, but want to provide a last chance for feedback in case there's a new concern that hasn't ever come up at Meta (which happened!) We also used them as a way to communicate what the feature is—essentially its initial docs.

As React started moving into a direction of being a multi-company project, incubating new features had to be open to the broader community — you see them available to frameworks for "incubation" too. So the moment you start "hearing" about a feature is much earlier now. The comparable situation at FB was when a couple of teams used a new feature. By that point it barely has any docs cause at first someone just needs to plug it into its first potential usage to see if the feature even works or makes sense. The big difference here is that we receive a lot of external feedback on these features from people who try to use them much earlier now.

By the time we get to the stage where we'd previously publish an RFC, we've already received so much structured feedback from people having used it that we haven't prioritized filing RFCs, slipped a few times, and then tried to figure out if we can find a better process that provides clarity but keeps the feedback focused and actionable. The "canary" docs we have on the React site (which we now try to update as soon as something reasonable lands on main) act as sort of a living public RFC, as we receive feedback from the users of those APIs as well as questions about them quite a lot. This usage feedback has led to changes.

This summer, we also wanted to tighten up our process in general, and try something closer to the TC39 staging system. Our process used to be more ad hoc — but now the team has grown enough that it's hard for everyone on the team to keep track of all the streams of work. As a reminder, this process has always been internal to the React team — so we're not speaking of closing something down; quite the opposite — the intention is to make those stages and the proposals moving between them public. The team was working on this in early August before pivoting to finish documenting the APIs already in Canaries, which was the most urgent feedback we've received about the community about what's missing. That work is just finishing now so I expect with some time, the team will formalize this stage-based process that it's currently piloting.

As for this specific change, the community feedback on the previous name "offscreen" was overwhelmingly that people thought it has something to do with things literally being offscreen (like under the viewport). That was not what the feature was originally (and is currently) doing — it was for marking some parts of the app as essentially inactive, for example, a tab you moved away from or a history stack entry. So it's "offscreen" in the sense of "we might have some state and DOM for it stashed away but it's not being explicitly displayed to the user in the current tree". So that's something more like "inert" than "offscreen". However, we also started using this feature in a more generic way, e.g. for pre-rendering experiments on React Native. There, the purpose is to render the components ahead of time (before the user actually opens the tab) but only spending CPU on that when otherwise idle and delaying all Effects from running. So it's kind of like "prewarming". We also talked about other potential patterns where e.g. the modal becomes "active" and the stuff below it becomes more "inactive" (state updates outside the modal can be deprioritized compared to updates inside the modal). There was also more discussion about how this feature seems like it could be the way to control progressive hydration (which currently requires wrapping into Suspense even when it doesn't make sene). As well as something that could be integrated with content-visibility. So overall there is a very relatively broad set of features here and it's all related to what the user is currently doing, i.e. what part of the screen is "active" in what sense, or which "activity" the user is engaged in.

The naming is not set in stone, but the old one was definitely wrong, and I the intention from the PR authors was to update it to our next best guess for what it might be called. I don't think it currently has the same API as the latest idea so this wasn't meant to look like a final call on the name or the behavior or its prop names (which might be outdated). This PR is a partial rename of a feature that changed in scope a few times, from the Meta side of the team that is currently dogfooding this feature (and providing feedback on its design to the broader React team). Unreleased APIs can churn like this many times before someone writes up a proper public-visible doc for it that's worth discussing and is able to engage with the feedback. I get that this is frustrating but it's not different at our level of commitment to features that are not yet shipped in Canaries.

Hope this helps. Sorry for super long post but I thought there's a lot to go over here.

@petetnt
Copy link
Contributor

petetnt commented Nov 8, 2023

Thanks for the response @gaearon, I think most people appreciate your longer answers!

@bvaughn
Copy link
Contributor

bvaughn commented Nov 8, 2023

That perspective on the rename makes sense. Thanks, Dan.

@kylvia
Copy link

kylvia commented Dec 6, 2023

Hello, may I ask when will this feature release the canary version?

bvaughn added a commit to replayio/devtools that referenced this pull request Feb 13, 2024
This should allow us to remove the `react-prod-sourcemaps` Webpack plug-in, which is a step toward unblocking a potential Webpack to Turbo migration.

I think the biggest change on the React side was renaming `unstable_Offscreen` to `unstable_Acitivy` -> facebook/react#27640

- [x] Update `react` and `react-dom` packages
  - [x] Replace all `unstable_Offscreen` references with `unstable_Activity`
- [x] Update `next.config` to include `react` and `react-dom` source-maps in bundle
  - [x] Remove `@acemarke/react-prod-sourcemaps` Webpack plug-in
  - [x] Verify production deploy (still) has source-maps for `react` and `react-dom`
- [x] Update `breakpoints-01` golden recording to one created from this PR
  - [x] Update `logpoints-10` and `logpoints-11` tests as needed to account for line number changes
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
…7640)

`Activity` is the current candidate name. This PR starts the rename work
by renaming the exported unstable component name.

NOTE: downstream consumers need to rename the import when updating to
this commit.
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
`Activity` is the current candidate name. This PR starts the rename work
by renaming the exported unstable component name.

NOTE: downstream consumers need to rename the import when updating to
this commit.

DiffTrain build for commit ce2bc58.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants