Skip to content

Conversation

wo-o29
Copy link
Contributor

@wo-o29 wo-o29 commented Sep 15, 2025

Overview

  1. Modifying the initial values ​​of the buildContext example document

In the example, null is passed as the second argument, but since the signature of buildContext is as follows, null cannot be passed.

function buildContext<ContextValuesType extends object>(
  contextName: string,
  defaultContextValues?: ContextValuesType, // null cannot be inserted
)

Also, since we used the <{ title: string }> generic type, we cannot initialize it with null.

  1. Absence of displayName from Provider

Currently, it is difficult to debug because there is no displayName for Provider, so the displayName is specified using the contextName prop.

Before After
스크린샷 2025-09-15 오후 7 39 38 스크린샷 2025-09-15 오후 7 40 03
  1. Fix branching of the useInnerContext function

Since the type declaration of Context is as follows, when reading the Context with useContext, it is inferred as ContextValuesType | undefined.

const Context = createContext<ContextValuesType | undefined>(
  defaultContextValues,
);

However, if there are no values ​​received as props within the Provider, null is returned. If props exist, the received props are returned.

const value = useMemo(
  () => (Object.keys(contextValues).length > 0 ? contextValues : null),
  // eslint-disable-next-line react-hooks/exhaustive-deps
  [...Object.values(contextValues)]
) as ContextValuesType;

Therefore, when reading useContext, ContextValuesType | null is actually returned, but the type system infers ContextValuesType | undefined.

To solve this complex and tangled type problem, the type of context was unified to ContextValuesType | null so that only null can be handled.

// as-is
const Context = createContext<ContextValuesType | undefined>(
  defaultContextValues,
);

const context = useContext(Context);
if (context != null) {
  return context;
}

// to-be
const Context = createContext<ContextValuesType | null>(defaultContextValues ?? null);

const context = useContext(Context);
if (context !== null) {
  return context;
}

Since the type of defaultContextValues ​​is <ContextValuesType extends object> | undefined , null cannot be provided, so there is no need to handle the case of null.

// as-is
if (defaultContextValues != null) {
  return defaultContextValues;
}

// to-be
if (defaultContextValues !== undefined) {
  return defaultContextValues;
}

Checklist

  • Did you write the test code?
  • Have you run yarn run fix to format and lint the code and docs?
  • Have you run yarn run test:coverage to make sure there is no uncovered line?
  • Did you write the JSDoc?

@Copilot Copilot AI review requested due to automatic review settings September 15, 2025 11:26
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request refactors the buildContext utility function to improve type safety and debugging experience. The changes address issues with null handling in TypeScript generics and enhance developer experience with better Provider naming.

  • Fixed TypeScript type inconsistencies by standardizing on null instead of mixing null and undefined
  • Added displayName to Provider components for better debugging in React DevTools
  • Updated documentation examples to use proper default values instead of null

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/utils/buildContext/buildContext.tsx Core implementation with type safety fixes and displayName addition
src/utils/buildContext/buildContext.md Updated English documentation example
src/utils/buildContext/ko/buildContext.md Updated Korean documentation example
Comments suppressed due to low confidence (1)

src/utils/buildContext/buildContext.tsx:17

  • The JSDoc example still shows null as the second parameter, but this is now invalid according to the updated function signature. This should be updated to match the corrected examples in the documentation files, such as { title: '' }.
 * const [Provider, useContext] = buildContext<{ title: string }>('TestContext', null);

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (5bdb4ff) to head (a98922c).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #277   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           37        37           
  Lines         1093      1094    +1     
  Branches       324       324           
=========================================
+ Hits          1093      1094    +1     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@wo-o29 wo-o29 changed the title Refactor/build context refactor(buildContext): added displayName and fixed useInnerContext branch handling Sep 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants