Skip to content

JSX preserve mode: make is not a valid component name #7432

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

Open
cknitt opened this issue May 6, 2025 · 4 comments
Open

JSX preserve mode: make is not a valid component name #7432

cknitt opened this issue May 6, 2025 · 4 comments
Assignees
Labels
Milestone

Comments

@cknitt
Copy link
Member

cknitt commented May 6, 2025

Consider a module Test.res with the following code:

let context = React.createContext(0)

module ContextProvider = {
  let make = React.Context.provider(context)
}

@react.component
let make = (~children) => {
  <ContextProvider value=42> {children} </ContextProvider>
}

Without JSX preserve mode enabled, the compiler output is (correct):

function Test(props) {
  return JsxRuntime.jsx(make, {
    value: 42,
    children: props.children
  });
}

With JSX preserve mode:

function Test(props) {
  return <make value={42}>
  {props.children}
  </make>;
}

However, make is not a valid React component name as it is not uppercase.

@cknitt cknitt added this to the v12 milestone May 6, 2025
@cknitt cknitt added the bug label May 6, 2025
@nojaf
Copy link
Collaborator

nojaf commented May 6, 2025

Hmm, the current React create context approach feels like a departure from the norm. It seems more like a workaround than a suitable solution.

React.createContext(0) should create a module with Provider and Consumer, but React.Context.provider returns a component definition.

To better fit into the existing component flow, it could be structured like this:

let context = React.createContext(0)

module ContextProvider = {
  @react.componentWithProps("Context")
  let make = props =>
    // Use React.jsxs or React.jsxKeyed accordingly
    React.jsx(context->React.Context.Provider, props)
}

module Foo = {
  @react.component
  let make = (~children) => {
    <ContextProvider value=42> {children} </ContextProvider>
  }
}

I'm not sure if this is worth fixing. I believe it should be addressed at its root; the functionality of createContext should be different, in my opinion.

@nojaf
Copy link
Collaborator

nojaf commented May 6, 2025

Something along these lines would fit the current flow better:

module MakeContext = (
  T: {
    type value
    let value: value
  },
) => {
  %%private(let context = React.createContext(T.value))

  module Consumer = {
    @send
    external consumer: (
      React.Context.t<T.value>,
      JsxDOM.domProps,
    ) => React.element = "Consumer"

    let make = {
      let \"Consumer" = props => consumer(context, props)
      \"Consumer"
    }
  }

  module Producer = {
    @send
    external producer: (
      React.Context.t<T.value>,
      React.Context.props<T.value>,
    ) => React.element = "Producer"

    let make = {
      let \"Producer" = props => producer(context, props)
      \"Producer"
    }
  }
}

module MyContext = MakeContext({
  type value = int
  let value = 0
})

module Bar = {
  @react.component
  let make = (~children) => {
    <MyContext.Producer value=42> {children} </MyContext.Producer>
  }
}

@cknitt
Copy link
Member Author

cknitt commented May 6, 2025

Hmm, the current React create context approach feels like a departure from the norm. It seems more like a workaround than a suitable solution.

React.createContext(0) should create a module with Provider and Consumer, but React.Context.provider returns a component definition.

That may be a better/more idiomatic API. Also note this change in React 19 BTW: https://react.dev/blog/2024/12/05/react-19#context-as-a-provider

However, even with different context bindings, couldn't there be other similar situations where one ends up with <make ... />?

@cknitt
Copy link
Member Author

cknitt commented May 6, 2025

E.g., simple case, when not using the PPX transform:

module X = {
  type props = {}
  let make = (_props: props) => React.null
}

@react.component
let make = () => <X />

compiling to

function make(_props) {
  return null;
}

let X = {
  make: make
};

function Test(props) {
  return <make/>;
}

let make$1 = Test;

Is emitting make instead of X.make here maybe an optimization that could somehow be disabled in this case (within JSX)? @cristianoc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants