Fable: importAll and importSideEffects shouldn't output inline values when used without a binding (Fable 3)

[Zaid-Ajaj]

Description

Trying to migrate a safe template application to Fable 3 and already coming across some weird issues when I tried to import a stylesheet into the frontend application. I already had:

importAll "./styles/main.scss"

Which used to work quite well and now it doesn't anymore because of the generated import:

import * as main from "./styles/main.scss"
main;

Notice how the import expression didn't only generate an import statement but also printed "main" on call-site. This is proving to be problematic when using parcel. I already filed an issue on parcel's repository.

I realized that maybe using importAll wasn't the best idea in the first place because it is about importing a module with a bunch of members and I considered using importSideEffects instead. The latter gave better results:

import "./styles/main.scss"
null;

However, it still outputs null on call-site and it seems like bundlers are tripping over it during development. I also noticed that these "side-effectful" imports generate statements that pushed before module imports, I am not sure whether this is a problem.

Expected and actual results

Would it be possible to:

• Make importAll behave the same as importSideEffects when used without a binding (i.e. just `importAll './file.css')? so that things keep working
• Not output anything on callsite of importAll and importSideEffects?
• Edit: put side-effectful imports _after_ module imports

Related information

• Fable version: 3.0.0-nagareyama-beta-002
• Operating system Windows 10

[alfonsogarciacaro]

Hmm, I just checked the REPL and it seems this is actually the same code that is generated by Fable 2. So we need to check if this is a Nagareyama regression or just different behavior between bundlers. I do have noticed that Webpack is more permissive with namespace imports, for example import * works with commonjs modules that export a single function like module.exports = function () {...} while other bundlers/transpilers consider this a "default" export.

For this reason, I would be careful of trying to turn importAll into importSideEffects depending on the context, as this may cause other kind of problems. About the second point, I guess we need to check and skip the non-productive statements for the (private) module declarations as we do with the blocks.

I also noticed that these "side-effectful" imports generate statements that pushed before module imports

Sorry, not sure if I understand this exactly 😅 If you mean the "side-effectful" import appears on top, I think right now Fable, besides grouping imports for the same file, just orders them as it founds the imports through the code. Not sure if the declaration order has an effect either, but if necessary I guess we could reorder them here (this is the function that processes the imports collected after transforming the file): https://github.com/fable-compiler/Fable/blob/7c52608e52f9decf7d95ca729a1f7082fac48cc5/src/Fable.Transforms/Fable2Babel.fs#L2019

[Zaid-Ajaj]

For this reason, I would be careful of trying to turn importAll into importSideEffects depending on the context, as this may cause other kind of problems. About the second point, I guess we need to check and skip the non-productive statements for the (private) module declarations as we do with the blocks.

Yeah you are right, I think the problem really was the printing of values like main or null in the middle of the file. This is also what people from parcel have reported.

Sorry, not sure if I understand this exactly 😅 If you mean the "side-effectful" import appears on top, I think right now Fable, besides grouping imports for the same file, just orders them as it founds the imports through the code. Not sure if the declaration order has an effect either, but if necessary I guess we could reorder them here (this is the function that processes the imports collected after transforming the file):

Sorry about the articulation, this is indeed what I meant. I would love if side-effectful imports happened after module/member imports because that is usually how JS code looks like and it would be good to make the code readable and familiar to JS people before I start spamming tooling projects with the JS code generated by Fable and asking why their tools don't work Fable 😁

On a related side-note. Is it possible to exportDefault without any changes to the value being exported? Right now function components are being transformed when you say:

[<ReactComponent>]
let Counter() = 
  let (count, setCount) = React.useState 0
  Html.div count

exportDefault Counter

Translates to

export default (() => createElement(Counter, null))

Is it possible to get instead?

export default Counter

[alfonsogarciacaro]

Perfect, yeah, these two things should be easy to fix. I already removed the stranded main and I will put now the side-effects import after the other ones.

About the export default, this is tricky. exportDefault is just Emit("export default $0") in disguise, and a couple of things are happening here: first the F# compiler wraps the member reference in a lambda as usual, and then the plugin kicks in and transforms the call into createElement... Not sure what would be the best fix/workaround. Maybe adding a exportDefault flag to the plugin and letting the plugin emit the export default directly? (for that we would have to change the plugin so it can output any declaration.

BTW, what time do you wake up? ;)

[Zaid-Ajaj]

About the export default, this is tricky. exportDefault is just Emit("export default $0") in disguise, and a couple of things are happening here: first the F# compiler wraps the member reference in a lambda as usual, and then the plugin kicks in and transforms the call into createElement... Not sure what would be the best fix/workaround. Maybe adding a exportDefault flag to the plugin and letting the plugin emit the export default directly? (for that we would have to change the plugin so it can output any declaration.

If we could override another member from the plugin, say TransformDefaultExport that would be awesome so that the plugin decides how the value should be exported without the extra wrapping. This is how Gastby and Next.js love to export their components. One problem though is that once we say "exportDefault Counter" and import it dynamically from another file, the plugin won't kick in (at least what I am guessing right now) but I think Gatsby and Next.js don't ask for importing components and they do it internally

BTW, what time do you wake up? ;)

Lol I don't know, my sleep rhythm is basically non-existent :joy:

[alfonsogarciacaro]

Ok, I've implemented two of the things discussed:

• Remove values generated by imports when they're not used (basically what I did was just to use the same filter we have for unproductive statements in the root scope).
• Put empty imports for side-effects after other static imports

I will add the ExportDefault option to the plugin in a later PR

[Zaid-Ajaj]

Awesome stuff @alfonsogarciacaro, the imports look much better now :smile:

Now I need to figure out why fast-refresh isn't working but it doesn't look like a Fable problem

I will add the ExportDefault option to the plugin in a later PR

Sweet! Please ping me when you do 🙏

[alfonsogarciacaro]

I was not very convinced of the solution, but after trying different approaches at the end I've added ExportDefault to MemberDecl. Please see a usage example here, let me know how it goes :) https://github.com/fable-compiler/fable-react/commit/ce20cee28e78ac270f691fc79b5c40fe9212b39e

[Zaid-Ajaj]

Looks like we both had the same line of thought (I was commenting on the commit) I think this will be OK-ish, it will take awhile before I can test this with Gatsby/Next.js because now focusing on getting fast-refresh to work with Feliz which is 90% working but currently figuring out the edge cases but I will definitely get back to ASAP