Fable: Inlined default imports broken in RC-08

Created on 20 Nov 2020  路  12Comments  路  Source: fable-compiler/Fable

Description

After #2280 was fixed, default imports now behave incorrectly.

I had this code:

module Stylesheet =

    type IStylesheet =
        [<Emit "$0[$1]">]
        abstract Item : className:string -> string

    /// Loads a CSS module and makes the classes within available
    let inline load (path: string) = importDefault<IStylesheet> path

Which is essentially an alias to importDefault with a nice indexer. This worked fine when I used it in RC-07:

let stylesheet = Stylesheet.load "./styles/main.css"
let container = stylesheet.["container"]

compiled to

import main from "./styles/main.css";
export const stylesheet = main;
export const container = stylesheet["container"];

However, in Fable 3 RC-08, the generated code looks like this:

import main from "./styles/main.css";
export const stylesheet = main("./styles/main.css")
export const container = stylesheet["container"];

Which breaks because main is not a function 馃槩

I am using a stylesheet to show the example but I believe any JS file exporting an object should replicate this behaviour.

Expected and actual results

Expected the code to not break: i.e. not passing the import path the to already imported function.

Related information

  • Fable version: 3 rc-08
  • Operating system: Windows

Most helpful comment

The bad news is I broke the plugin 馃槄 Sorry!

No worries, Alfonso! I know this is an expected part of the feature and I don't mind because the net benefit has been tremendous :smile: I just published Feliz v1.18 which is using the latest plugin goodies. Thanks a lot for the reference and comments in the PR, it was pretty straightforward to make.

All 12 comments

Stop inlining the imports! ...just kidding 馃槈 This is tricky because of the weird way the import helpers consume the arguments. In the previous issue we didn't want it to absorb them but now that's actually what we need. I've pushed a solution to tell both cases apart, though it won't cover weird uses, like using the same parameter in the import expression AND as argument of the imported function, so please don't do that 馃槉

Before making a new release, can you please check that everything works and I haven't broken anything else again? To test a local Fable build you need to:

  • Clone Fable repo and pull latest changes in nagareyama (the default branch now)
  • Install dependencies with "npm install" and build fable-library: "npm run build library"
  • In your project, instead of "dotnet fable" use "dotnet run -c Release -p ../Fable/src/Fable.Cli --". Note you may have to adjust the path depending on your directory structure and the "--" before the Fable arguments.

Stop inlining the imports! ...just kidding 馃槈 This is tricky because of the weird way the import helpers consume the arguments. In the previous issue we didn't want it to absorb them but now that's actually what we need.

It is really nice to work with though from the user perspective like making a nice and simple API for imported stylesheet modules 馃榿

Besides dotnet run -c Release -p ../Fable/src/Fable.Cli -- anything pre-build step that I should execute to test out the branch?

Hehe, I was just kidding. I agree it makes for a nicer API and we cannot use the attributes here so it should be fixed :+1:

Ah, sorry. Forgot that you need to build the local fable-library. You can do that by typing "npm i" and "npm run build library"

Awesome, I will give it a try soon!

I don't know why but npm install hangs on my Windows 10 :/ especially on the preinstall event which I can't seem to find anywhere in package.json 馃槩

Maybe just publish another RC release and we test it that way? If the issue is still not resolved, I will try harder to get Fable to work on my machine 馃

Oh, that's weird, maybe you can try with yarn if you've it installed. I won't have access to my computer until Monday. I can give you ownership of the package if you need it before, but then you need to be able to build :/

After a restart,npm install worked. However, I had to make several changes to the code to actually make it compile. For example, inside build.fsx there is npx tsc ... but in the package.json there is typescript and both seem to be different packages after a quick --version check 馃槥 the tsc version couldn't parse the tsconfig.json because of the comments and the provided options weren't recognized by it until I replaced tsc with typescript in build.fsx. Maybe consider not using npx and only npm to make sure the commands executed are using locally installed packages as specified by package.json and its associated lock file.

In any case, I eventually was able to run the Fable against my local project and can confirm that the fix actually resolved the problem at hand 馃帀

Great @Zaid-Ajaj, thanks a lot for confirming and for pointing out the issue with npx. Makes sense to replace it to avoid problems as yours, I will do it 馃憤

Hmm, now that I'm checking. In my machine there's no tsc package. It just happens that this is the command for the typescript package, and if I type npx tsc --version the correct version (3.9.3) is shown. I don't have typescript installed globally 馃

The problem with using "npm run" instead of "npx" is that you can only run scripts in the package.json, not the commands in node_modules/.bin directly.

Maybe it is a windows problem because I got something like 1.2x for tsc and 3.9x for typescript

@Zaid-Ajaj The good news is I've pushed a new release with the fix. The bad news is I broke the plugin 馃槄 Sorry! I used my last card and it won't happen anymore until the official release. Please check this for instructions to upgrade the plugin, it's only a few small changes: https://github.com/fable-compiler/fable-react/pull/207/commits/ffcce83c65c3fe64d7ebcc258b2368bed6705cbc

EDIT: I added some comments but seems they are not shown in the link above, you can also try this one: https://github.com/fable-compiler/fable-react/pull/207#pullrequestreview-538803888

The bad news is I broke the plugin 馃槄 Sorry!

No worries, Alfonso! I know this is an expected part of the feature and I don't mind because the net benefit has been tremendous :smile: I just published Feliz v1.18 which is using the latest plugin goodies. Thanks a lot for the reference and comments in the PR, it was pretty straightforward to make.

Was this page helpful?
0 / 5 - 0 ratings