Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
• Changed tests to use pragma
• Renamed feature flag
  • Loading branch information
Brian Vaughn committed Oct 19, 2020
1 parent 32e5c1b commit 551985a
Show file tree
Hide file tree
Showing 13 changed files with 45 additions and 42 deletions.
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberHooks.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {
enableNewReconciler,
decoupleUpdatePriorityFromScheduler,
enableDoubleInvokingEffects,
enableUseRefMutationWarning,
enableUseRefAccessWarning,
} from 'shared/ReactFeatureFlags';

import {
Expand Down Expand Up @@ -1216,7 +1216,7 @@ function getCallerStackFrame(): string {

function mountRef<T>(initialValue: T): {|current: T|} {
const hook = mountWorkInProgressHook();
if (enableUseRefMutationWarning) {
if (enableUseRefAccessWarning) {
if (__DEV__) {
// Support lazy initialization pattern shown in docs.
// We need to store the caller stack frame so that we don't warn on subsequent renders.
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberHooks.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
enableSchedulingProfiler,
enableNewReconciler,
decoupleUpdatePriorityFromScheduler,
enableUseRefMutationWarning,
enableUseRefAccessWarning,
} from 'shared/ReactFeatureFlags';

import {NoMode, BlockingMode, DebugTracingMode} from './ReactTypeOfMode';
Expand Down Expand Up @@ -1194,7 +1194,7 @@ function getCallerStackFrame(): string {

function mountRef<T>(initialValue: T): {|current: T|} {
const hook = mountWorkInProgressHook();
if (enableUseRefMutationWarning) {
if (enableUseRefAccessWarning) {
if (__DEV__) {
// Support lazy initialization pattern shown in docs.
// We need to store the caller stack frame so that we don't warn on subsequent renders.
Expand Down
59 changes: 31 additions & 28 deletions packages/react-reconciler/src/__tests__/useRef-test.internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ describe('useRef', () => {

const ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
ReactFeatureFlags.enableUseRefMutationWarning = true;

act = ReactNoop.act;
useCallback = React.useCallback;
Expand Down Expand Up @@ -104,32 +103,6 @@ describe('useRef', () => {
expect(Scheduler).toHaveYielded(['ping: 6']);
});

it('should never warn when attaching to children', () => {
class Component extends React.Component {
render() {
return null;
}
}

function Example({phase}) {
const hostRef = useRef();
const classRef = useRef();
return (
<>
<div key={`host-${phase}`} ref={hostRef} />
<Component key={`class-${phase}`} ref={classRef} />
</>
);
}

act(() => {
ReactNoop.render(<Example phase="mount" />);
});
act(() => {
ReactNoop.render(<Example phase="update" />);
});
});

it('should return the same ref during re-renders', () => {
function Counter() {
const ref = useRef('val');
Expand All @@ -155,6 +128,33 @@ describe('useRef', () => {
});

if (__DEV__) {
it('should never warn when attaching to children', () => {
class Component extends React.Component {
render() {
return null;
}
}

function Example({phase}) {
const hostRef = useRef();
const classRef = useRef();
return (
<>
<div key={`host-${phase}`} ref={hostRef} />
<Component key={`class-${phase}`} ref={classRef} />
</>
);
}

act(() => {
ReactNoop.render(<Example phase="mount" />);
});
act(() => {
ReactNoop.render(<Example phase="update" />);
});
});

// @gate enableUseRefAccessWarning
it('should warn about reads during render', () => {
function Example() {
const ref = useRef(123);
Expand Down Expand Up @@ -215,7 +215,8 @@ describe('useRef', () => {
});
});

it('should not warn about unconditional lazy init during render', () => {
// @gate enableUseRefAccessWarning
it('should warn about unconditional lazy init during render', () => {
function Example() {
const ref1 = useRef(null);
const ref2 = useRef(undefined);
Expand Down Expand Up @@ -255,6 +256,7 @@ describe('useRef', () => {
});
});

// @gate enableUseRefAccessWarning
it('should warn about reads to ref after lazy init pattern', () => {
function Example() {
const ref1 = useRef(null);
Expand Down Expand Up @@ -288,6 +290,7 @@ describe('useRef', () => {
});
});

// @gate enableUseRefAccessWarning
it('should warn about writes to ref after lazy init pattern', () => {
function Example() {
const ref1 = useRef(null);
Expand Down
2 changes: 1 addition & 1 deletion packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,4 +135,4 @@ export const enableDiscreteEventFlushingChange = false;

export const enableDoubleInvokingEffects = false;

export const enableUseRefMutationWarning = false;
export const enableUseRefAccessWarning = false;
2 changes: 1 addition & 1 deletion packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export const decoupleUpdatePriorityFromScheduler = false;
export const enableDiscreteEventFlushingChange = false;

export const enableDoubleInvokingEffects = false;
export const enableUseRefMutationWarning = false;
export const enableUseRefAccessWarning = false;

// Flow magic to verify the exports of this file match the original version.
// eslint-disable-next-line no-unused-vars
Expand Down
2 changes: 1 addition & 1 deletion packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export const decoupleUpdatePriorityFromScheduler = false;
export const enableDiscreteEventFlushingChange = false;

export const enableDoubleInvokingEffects = false;
export const enableUseRefMutationWarning = false;
export const enableUseRefAccessWarning = false;

// Flow magic to verify the exports of this file match the original version.
// eslint-disable-next-line no-unused-vars
Expand Down
2 changes: 1 addition & 1 deletion packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export const decoupleUpdatePriorityFromScheduler = false;
export const enableDiscreteEventFlushingChange = false;

export const enableDoubleInvokingEffects = false;
export const enableUseRefMutationWarning = false;
export const enableUseRefAccessWarning = false;

// Flow magic to verify the exports of this file match the original version.
// eslint-disable-next-line no-unused-vars
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export const decoupleUpdatePriorityFromScheduler = false;
export const enableDiscreteEventFlushingChange = false;

export const enableDoubleInvokingEffects = false;
export const enableUseRefMutationWarning = false;
export const enableUseRefAccessWarning = false;

// Flow magic to verify the exports of this file match the original version.
// eslint-disable-next-line no-unused-vars
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export const decoupleUpdatePriorityFromScheduler = false;
export const enableDiscreteEventFlushingChange = false;

export const enableDoubleInvokingEffects = false;
export const enableUseRefMutationWarning = false;
export const enableUseRefAccessWarning = false;

// Flow magic to verify the exports of this file match the original version.
// eslint-disable-next-line no-unused-vars
Expand Down
2 changes: 1 addition & 1 deletion packages/shared/forks/ReactFeatureFlags.testing.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export const decoupleUpdatePriorityFromScheduler = false;
export const enableDiscreteEventFlushingChange = false;

export const enableDoubleInvokingEffects = false;
export const enableUseRefMutationWarning = false;
export const enableUseRefAccessWarning = false;

// Flow magic to verify the exports of this file match the original version.
// eslint-disable-next-line no-unused-vars
Expand Down
2 changes: 1 addition & 1 deletion packages/shared/forks/ReactFeatureFlags.testing.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export const decoupleUpdatePriorityFromScheduler = false;
export const enableDiscreteEventFlushingChange = true;

export const enableDoubleInvokingEffects = false;
export const enableUseRefMutationWarning = false;
export const enableUseRefAccessWarning = false;

// Flow magic to verify the exports of this file match the original version.
// eslint-disable-next-line no-unused-vars
Expand Down
2 changes: 1 addition & 1 deletion packages/shared/forks/ReactFeatureFlags.www-dynamic.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,4 @@ export const enableTrustedTypesIntegration = false;
export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;

export const enableDoubleInvokingEffects = false;
export const enableUseRefMutationWarning = false;
export const enableUseRefAccessWarning = __VARIANT__;
2 changes: 1 addition & 1 deletion packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export const {
enableDebugTracing,
skipUnmountedBoundaries,
enableDoubleInvokingEffects,
enableUseRefMutationWarning,
enableUseRefAccessWarning,
} = dynamicFeatureFlags;

// On WWW, __EXPERIMENTAL__ is used for a new modern build.
Expand Down

0 comments on commit 551985a

Please sign in to comment.