This repository has been archived by the owner on Jul 19, 2019. It is now read-only.
Add notes for April 7 #3
There are no files selected for viewing
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,129 @@ | ||
| ## April 7 ([discuss](https://github.com/reactjs/core-notes/pull/2)) | ||
|
|
||
| ### Releasing React 15 | ||
|
|
||
| #### Shipping Today | ||
|
|
||
| * React 15 is shipping! 🎉 | ||
| * Docs should be in good shape. | ||
| * Just need to proofread the [blog post]([#6396](https://github.com/facebook/react/pull/6396)) and ship it. | ||
|
|
||
| #### Relay Test Failure | ||
|
|
||
| * We changed React to use `object-assign` npm [ponyfill](https://ponyfoo.com/articles/polyfills-or-ponyfills) that falls back to native `Object.assign`. ([#6376](https://github.com/facebook/react/pull/6376)) | ||
| * This caused a Relay test to fail internally because native `Object.assign` did not preserve key ordering. | ||
| * Test will be fixed to not rely on this. | ||
| * This won’t block React 15 release. | ||
|
|
||
| #### React Native Issue | ||
|
|
||
| * `react-native init` install `react@latest` which would break when we release React 15. | ||
| * Sorted out with [James Ide](https://twitter.com/ji). | ||
| * Since we communicate with [Exponent](https://twitter.com/exponentjs) frequently, we were able to fix this preemptively. | ||
| * We’ll release 15 but keep 0.14 `latest` on npm for a while (hopefully a few hours) until React Native gets a fix. | ||
|
|
||
| #### Branch Planning | ||
|
|
||
| * At a high level, things will be slightly different than they’ve been historically because we now have actual minor versions. | ||
| * We’ll need to maintain both a patch release and a minor release going forward, so we need to figure out how many releases we’re going to support officially. | ||
| * This was also a common question from the community (what is our official story around support of past releases?). | ||
| * Node does labels on every request, we’ll need to figure out what our strategy will be so we cherry-pick all the changes correctly. | ||
| * [Paul](https://twitter.com/zpao) will study how Node does it and come up with a plan. | ||
|
|
||
| #### Rolling Changelog | ||
|
|
||
| * Let’s maintain a continuously updated changelog so we don’t have to spend hours (sometimes days!) writing release blog entries. | ||
| * We need to figure out how to where to put “unreleased” entries, how to handle merge conflicts, etc. | ||
|
There was a problem hiding this comment. There are some good ideas in Keep a Changelog, especially regarding where unreleased entries go, and also arguing that changelog entries should be written by humans for humans rather than harvested from commit messages. There was a problem hiding this comment. Yeah. We do intend to keep writing them manually. Doing this during (bi)weekly sync to Facebook codebase sounds like a sensible idea to all of us so we will give this a try. |
||
| * Leaning towards having an oncall or whoever merges things updating the changelog. | ||
| * What if whenever we sync React master internally to Facebook (once a couple of weeks), we also generate the changelog? | ||
| * This means that whoever does the sync has to do a little more work. | ||
| * Combination of the batched approach + human discretion. | ||
| * Will start this process with the next internal sync. | ||
|
|
||
| ### Browser Tests | ||
|
|
||
| * React attempts to normalize behavior across browsers but we don’t have any browser tests. ([#5703](https://github.com/facebook/react/issues/5703)) | ||
| * We used to have them with Sauce Labs but they were super flaky and we removed them. ([#4393](https://github.com/facebook/react/pull/4393)) | ||
| * Most of our tests aren’t tied to Jest and we should be able to get them running in the browser again. | ||
| * Should we run the existing tests, or are there new types of things we want to test? | ||
| * The types of things we care about with browser tests are often different. | ||
| * Usually we don’t need Jest-isms like clearing the module registry in these tests as they only work with public API. | ||
| * Example: Input selection should not jump to end of input when focusing a controlled input (or something like this). | ||
| * Should we be worried tests will get flaky again? | ||
| * We’ll start with a few very focused tests and see how it goes. | ||
| * Let’s decide on a test runner and write some tests. | ||
| * Let’s pick a recent regression and try to write a test for it. | ||
| * [Jim](https://github.com/jimfb) will work on this. | ||
|
|
||
|
|
||
| ### Attributes vs Properties | ||
|
|
||
| * In React 15, we switched to using DOM attributes by default and using properties only in special cases. | ||
| * This fixed a few issues but then introduced a few other issues. | ||
| * While we patched over those regressions and 15 looks good, there is no consensus yet on whether this was the right call. | ||
|
|
||
| #### Reasons to Use Attributes over Properties | ||
|
|
||
| * Attributes currently match our desired behavior for more or less all supported props, including SVG. In the future properties might support more structured data but there’s nothing there we _currently_ want to take advantage of. | ||
| * 0.14 already created HTML strings on initial render, and attributes are the closest alternative. | ||
| * If we use attributes for everything we could just drop the whitelist completely (and treat only form components specially), which we may or may not want to do but it seems appealing. | ||
| * For the specific regression that relying on attributes introduced ([not setting `value=""` on option tags](https://github.com/facebook/react/issues/6219)), we _can’t_ use properties because there is a semantic difference between `value=""` and no attribute, but `.value` is `''` in both cases. | ||
| * We’re not currently invested into designing a rich API for React DOM props right now so less maintenance burden there seems beneficial. | ||
|
There was a problem hiding this comment. (It's because we're not actively doing this that less maintenance and fewer decisions is appealing.) |
||
|
|
||
| #### Reasons to Use Properties over Attributes | ||
|
|
||
| * Attributes seem to only match our wanted behavior for “metadata”, i.e. things that the DOM doesn’t actually really care about. The exception is SVG. | ||
| * Attributes for initial render is a waste of resources (`toString` / `parse`) but semantically is OK. They also make sense for server rendering since the point of attributes is to provide an initial value. However this is not a good argument for why they should be used for values that change over time. | ||
| * The addition of new elements and features leads to more controlled values and richer data types. Attributes are the legacy, not properties. | ||
| * If there are any semantic differences that depend on attributes, we should flag those to standard committees. Then we can polyfill those using attributes, and then eventually remove the fix. | ||
| * Maybe we should find someone to actually design this rich API mapping for React DOM props and maintain it. We aren’t busy with this now but this doesn’t mean the problem isn’t worthwhile. Hopefully one day it might be standardized in some form or just considered the canonical declarative bindings to the DOM. | ||
|
|
||
| #### Conclusion | ||
|
|
||
| * No conlusion yet. We plan to come back to this next week. | ||
|
There was a problem hiding this comment. Mind sending a pull request? Just open the note in master and "edit". Thanks! |
||
| * React 15 uses attributes. | ||
| * The regressions in RCs appear fixed. | ||
|
|
||
| #### Relevant Issues and Pull Requests | ||
|
|
||
| * [#1431](https://github.com/facebook/react/issues/1431) | ||
| * [#1510](https://github.com/facebook/react/pull/1510) | ||
| * [#4618](https://github.com/facebook/react/issues/4618) | ||
| * [#5666](https://github.com/facebook/react/pull/5666) | ||
| * [#5680](https://github.com/facebook/react/pull/5680) | ||
| * [#5907](https://github.com/facebook/react/pull/5907) | ||
| * [#5966](https://github.com/facebook/react/issues/5966) | ||
| * [#6119](https://github.com/facebook/react/issues/6119) | ||
| * [#6219](https://github.com/facebook/react/issues/6219) | ||
| * [#6228](https://github.com/facebook/react/pull/6228) | ||
| * [#6406](https://github.com/facebook/react/pull/6406) | ||
|
|
||
| ### Recent Pull Requests | ||
|
|
||
| #### `ReactDOM.render()` return value being <s>deprecated</s> legacy ([#6400](https://github.com/facebook/react/pull/6400)) | ||
|
|
||
| * It’s not deprecated yet and we don’t have a solid plan. But new code should avoid it. | ||
| * We still rely on it heavily in our tests (basically all of them!) | ||
| * We might need a separate reconciler for tests anyway. | ||
| * We want to encourage people to move away from it because, if we implement incremental reconciler, we might not be able to always resolve nodes synchronously. | ||
| * The only thing we know for sure is that this is a potential stumbling block going forward. | ||
|
|
||
| #### [Sebastian](https://twitter.com/sebmarkbage) is moving some files from React Native to React ([#6338](https://github.com/facebook/react/pull/6338)) | ||
|
|
||
| * This adds a new package called `react-native-renderer`. | ||
| * It doesn’t block 15 and won’t need to wait until 16. | ||
| * We might also split out the `react-dom-renderer` from `react-dom` in the future. | ||
|
There was a problem hiding this comment. Any information why is it needed? There was a problem hiding this comment. cc @sebmarkbage who has more context on this There was a problem hiding this comment. @artyomtrityak @gaearon I think that's might be the reason There was a problem hiding this comment. No, I don't think so. Separating factories is something we intend to do, but it is not related to separating the renderer. I know this is confusing. We intend to document internal terminology soon. There was a problem hiding this comment. Hmm.. I was assuming that those factories should go to There was a problem hiding this comment. This was addressed here: facebook/react#6169 (comment). Ideally factories should be moved to a completely different package because they aren't used by most people and don't actually depend on specific renderer. |
||
| * React Native repo will then contain components and the bridge, but not the parts that integrate tightly with React core. | ||
| * Are all React Native engineers going to start pushing to the React repository now? | ||
| * This is mostly stable React code, so shouldn’t churn much. | ||
| * But yes, this is something we’ll have to figure out. | ||
| * Waiting until Sebastian is back to provide an answer here. | ||
| * Why are we doing this? | ||
| * Ultimate goal is to make it so React core is separate from each of the renderers. | ||
| * Wherever there’s overlap, we want to be able to iterate and ship independently. | ||
| * A small change to React DOM should not necessarily trigger a new React release. | ||
| * Versioning | ||
| * We've started separating different packages, but they’re all still versioned at the same time. | ||
| * Is this going to change? Yes, but not right now. | ||
| * Again, we should be able to iterate on React core and downstream dependencies of React core independently. | ||
| * Lots left to figure out here. | ||
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maintaining the changelog is a pain, but maintaining changelogs for several release branches without strict "commit message guide" is an agony.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @zpao
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That definitely contributes in a way, but I think if you have to look at 10 changes once a week, you can form nice changelog entries. Commit messages help but we've mostly optimized to lower the burden of contribution by not enforcing anything there and forcing our humans to write messages for humans. I've contributed to projects with strict guidelines there and luckily I'm very familiar with rebasing and editing commits but not everybody is and it would exclude potential contributors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zpao as a person who really don't like any sort of routines I absolutely agree with your point. But at the same time, PRs in nodejs or angular shows that commit/PR guide is not so bad idea.
For me personally changelogs of nodejs are almost ideal. But nodejs has regular releases (1 per week or two), so they can keep changelogs lite and human readable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@narqo Is there something you'd like to see different in React changelogs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only thing I can think is to have links to PR/commit for every item. Sometimes it really helps to understand why/how/what happened. Especially from the contributers pov.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already coming in v15's changelog. :)