Zaid-Ajaj / Feliz

A fresh retake of the React API in Fable and a collection of high-quality components to build React applications in F#, optimized for happiness
https://zaid-ajaj.github.io/Feliz/
MIT License
540 stars 78 forks source link

Add `import React from "react"` automatically when the file contains a `[<ReactComponent>]` #510

Closed MangelMaxime closed 1 year ago

MangelMaxime commented 2 years ago

In order for React fast refresh to works, the following import import React from "react" needs to be in the file.

Demo:

https://user-images.githubusercontent.com/4760796/189533763-097ee58f-8efc-438a-9b07-be3ed2c2b347.mp4

  1. When first editing the file see how the page does a full reload
  2. When I edit after adding import React, { createElement } from "react" the file is now using hot reload.

One workaround is to add

open Fable.Core.JsInterop

// Workaround to have React-refresh working
// I need to open an issue on react-refresh to see if they can improve the detection
emitJsStatement () "import React from \"react\""

at the top of each file but this is easy to forget and error prone. Especially, because the user don't need to add it on top of each file, but just the one which export React components.

@alfonsogarciacaro Is it possible to use the FelizComponent plugin to auto inject the line import React from "react" in the file?

I don't know if having several time the same import has any implication in JavaScript. But, we probably don't want to inject it at each FelizComponent occurrence but only once per file.

alfonsogarciacaro commented 2 years ago

Good point, I actually had a similar problem here

Fable groups imports before writing JS so duplication is not an issue. We just need to inject the import in the code, I can try to do that.

MangelMaxime commented 2 years ago

Would be really cool to have it 🙏

If possible on both the current version and next version if that's not too much work.

Like that we can clean up a bunch of manual import React from "react" and I can test it on my projects.

alfonsogarciacaro commented 2 years ago

Damn, I gave this a try but I found a problem. We can inject a default import but Fable assigns the variable name automatically so it results in something like this:

import react_1 from "react";

I can fix this in Fable so it allows a pattern like import "default as React" "react" but it will require users to update both Fable and the plugins so it may not make much sense for Fable 3. It could be another incentive for users to upgrade though.

MangelMaxime commented 2 years ago

This indeed ring a bell to me and that's probably why I used emitJsStatement as Fable just pass the provided value without modification.

If it is not possible to add to Fable 3, that's unfortunate but can indeed be one of the improvement made thanks to Fable 4 upgrade.