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

feat: add support for non-dev jsx runtime #224

Merged
merged 1 commit into from Sep 24, 2023

Conversation

jihchi
Copy link
Contributor

@jihchi jihchi commented Sep 23, 2023

Description

I stumble across an issue where the Fast Refresh feature doesn't work with a Vite plugin I'm maintaining. After examination, I identified the root cause: The generated code with .js extension uses react/jsx-runtime for JSX transformation instead of react/jsx-dev-runtime, because of this, the generated code is no longer eligible for applying Fest Refresh, which results in a full page reloaded.

Here is a JS file generated by the compiler, for example (click to expand)

Filename: Res.bs.js

// Generated by ReScript, PLEASE EDIT WITH CARE

import * as JsxRuntime from "react/jsx-runtime";

function Res(props) {
  return JsxRuntime.jsx("h3", {
    children: "ReScript Component",
    className: "text-blue-100",
  });
}

var make = Res;

export { make };
/* react/jsx-runtime Not a pure module */

In order to fix it, this PR adds an extra check for such case so that the Fast Refresh works as expected.


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the PR Title Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@jihchi jihchi changed the title feat: Add support for non-dev jsx runtime Add support for non-dev jsx runtime Sep 23, 2023
@jihchi jihchi force-pushed the add_support_jsx_non_dev_runtime branch from 0ddc6dd to ce86503 Compare September 23, 2023 12:44
@jihchi jihchi changed the title Add support for non-dev jsx runtime fear: add support for non-dev jsx runtime Sep 23, 2023
@jihchi jihchi changed the title fear: add support for non-dev jsx runtime feat: add support for non-dev jsx runtime Sep 23, 2023
@ArnaudBarre
Copy link
Member

Is there any reason why ReScript is not generating code with jsxDev? This could have some benefits like adding information the for react devtools and making it work with https://github.com/ArnaudBarre/vite-plugin-react-click-to-component

@jihchi
Copy link
Contributor Author

jihchi commented Sep 23, 2023

Hi @ArnaudBarre, I have no idea. I've done some searching but there's no insightful result. For what it's worth, it appears that it was implemented this way initially.

As far as I'm concerned, it is likely a challenge and substantial effort to make jsxDev works seamlessly in the compiler.

@ArnaudBarre
Copy link
Member

Thanks for the great PR. I will merge this as there is also other cases where this can show up vitejs/vite-plugin-react-pages#132

@ArnaudBarre ArnaudBarre merged commit 25fe88a into vitejs:main Sep 24, 2023
8 of 10 checks passed
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.

None yet

2 participants