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

Upgrade to Nagareyama #43

Merged
merged 6 commits into from
Mar 10, 2021

Conversation

alfonsogarciacaro
Copy link
Contributor

@alfonsogarciacaro alfonsogarciacaro commented Oct 5, 2020

EDIT: Source maps are back in Fable 3.1, check this comment below for additional instructions.

To upgrade a project using Webpack to Nagareyama do the following steps:

  1. Make sure you have dotnet SDK 5 or newer installed. If necessary, run dotnet new tool-manifest in your repo.
  2. Set the last file of your project + .js as the entry in webpack.config.js (see how it's done in this PR)
  3. Run dotnet tool install fable
  4. For development (adjust the path to the .fsproj if necessary), run dotnet fable watch src/Client --run webpack-dev-server
  5. For production, run dotnet fable src/Client --run webpack

Cleanup

  • Add *.fs.js to .gitignore:
  • From package.json, remove "fable-loader" and "fable-compiler"
  • In webpack.config.js, remove the "fable-loader" rule

Notes

  • The --run argument must always come last, everything after it will be applied to the subprocess, e.g. dotnet fable src --run webpack --config my.config.js
  • By default, generated JS files will be next to F# sources with .fs.js extension. If you want to put them in another directory use --outDir, e.g. dotnet fable src --outDir build. When specifying an output directory, the default extension becomes .js. You can set the extension explicitly in any case with the --extension argument. Adjust the entry point in webpack.config.js accordingly. (Note: only .fs files will be moved to the output directory.)

@kerams
Copy link

kerams commented Oct 23, 2020

Can babel-loader go too?

@alfonsogarciacaro
Copy link
Contributor Author

It depends on your target, if you only need to target modern browsers I believe you can remove it. But if you need to be compatible with say, Internet Explorer, I think you still need babel-loader to inject the polyfills and transform the JS syntax.

@forki
Copy link

forki commented Oct 25, 2020

since this is the canonical upgrade sample. I suggest you add the optional but important --outDir parameter as suggestion

@forki
Copy link

forki commented Oct 27, 2020

what about Fable.Core and other libs. should we paket update those to prerelease channel?

@alfonsogarciacaro
Copy link
Contributor Author

Funny thing is Fable.Core has been v3 for a while :) The nice thing is most libs shouldn't need to do anything to compile with Nagareyama. There will likely be a Fable.Core 4 (pre)release for a couple of new features: mangled interfaces and emitJs functions, but they're quite minor so there's no rush to update. We can publish the package when writing the post explaining the features.

@danfma
Copy link

danfma commented Nov 25, 2020

Only one thing is missing (in the issue's description at least). You should change the webpack.config.js to point to the generated file instead of the App.fsproj.

@alfonsogarciacaro
Copy link
Contributor Author

It's the second point in the description above but maybe the wording is not good and it's easy to miss 🤔

@psfinaki
Copy link

psfinaki commented Dec 1, 2020

YarnInstall build step is still needed I guess?

@alfonsogarciacaro
Copy link
Contributor Author

Fable is installed now by "dotnet tool restore" (if it's already in the dotnet tools manifest), but you still need yarn (or npm) for JS tooling, like we pack, and other dependencies.

@halcwb
Copy link

halcwb commented Dec 20, 2020

Following the upgrade recipe, everything seems to work, only I get error messages from everything related to material ui theme functions like:

VM6191 log.js:24 [HMR] Waiting for update signal from WDS...
prelude.fs.js?bee5:10 Initial state: State {SelectedGeneric: undefined, SelectedIndication: undefined, SelectedRoute: undefined, SelectedPatient: undefined, Versions: Deferred$1, …}
getStylesCreator.js?0908:28 Uncaught TypeError: theme.Feliz.MaterialUI.Theme.spacingZ524259A4 is not a function
at eval (TitleBar.fs.js?c8cf:22)
at Object.create (getStylesCreator.js?0908:19)
at attach (makeStyles.js?443e:94)
at eval (makeStyles.js?443e:236)
at useSynchronousEffect (makeStyles.js?443e:188)
at useStyles (makeStyles.js?443e:228)
at titlebar (TitleBar.fs.js?c8cf:30)
at renderWithHooks (react-dom.development.js?61bb:14803)
at mountIndeterminateComponent (react-dom.development.js?61bb:17482)
at beginWork (react-dom.development.js?61bb:18596)
eval @ TitleBar.fs.js?c8cf:22
create @ getStylesCreator.js?0908:19
attach @ makeStyles.js?443e:94
eval @ makeStyles.js?443e:236
useSynchronousEffect @ makeStyles.js?443e:188
useStyles @ makeStyles.js?443e:228
titlebar @ TitleBar.fs.js?c8cf:30
renderWithHooks @ react-dom.development.js?61bb:14803
mountIndeterminateComponent @ react-dom.development.js?61bb:17482
beginWork @ react-dom.development.js?61bb:18596
callCallback @ react-dom.development.js?61bb:188
invokeGuardedCallbackDev @ react-dom.development.js?61bb:237
invokeGuardedCallback @ react-dom.development.js?61bb:292
beginWork$1 @ react-dom.development.js?61bb:23203
performUnitOfWork @ react-dom.development.js?61bb:22154
workLoopSync @ react-dom.development.js?61bb:22130
performSyncWorkOnRoot @ react-dom.development.js?61bb:21756
scheduleUpdateOnFiber @ react-dom.development.js?61bb:21188
updateContainer @ react-dom.development.js?61bb:24373
eval @ react-dom.development.js?61bb:24758
unbatchedUpdates @ react-dom.development.js?61bb:21903
legacyRenderSubtreeIntoContainer @ react-dom.development.js?61bb:24757
render @ react-dom.development.js?61bb:24840
eval @ react.fs.js?f36f:14
requestAnimationFrame (async)
setState @ react.fs.js?f36f:13
eval @ Util.js?e1e1:535
uncurriedFn @ Util.js?e1e1:500
mapSetState @ Main.fs.js?0c19:174
eval @ Util.js?e1e1:571
uncurriedFn @ Util.js?e1e1:500
ProgramModule_runWith @ program.fs.js?3f3e:152
eval @ Main.fs.js?0c19:205
eval @ Main.fs.js?0c19:22
./src/Informedica.Formulary.Client/Main.fs.js @ app.js:1921
webpack_require @ app.js:849
fn @ app.js:151
0 @ app.js:1982
webpack_require @ app.js:849
checkDeferredModules @ app.js:46
(anonymous) @ app.js:925
(anonymous) @ app.js:928
react-dom.development.js?61bb:19527 The above error occurred in the component:
in titlebar (created by Components_LazyView$1)
in div (created by Components_LazyView$1)
in ThemeProvider (created by Components_LazyView$1)
in Components_LazyView$1

@alfonsogarciacaro
Copy link
Contributor Author

There was an issue with Feliz.MaterialUI in Fable 3 but it should be fixed now. Are you using the latest version? If not, can you please try upgrading Feliz.MaterialUI?

@halcwb
Copy link

halcwb commented Dec 20, 2020

@alfonsogarciacaro Thanks for the quick reply! But unfortunately this hasn't been resolved in the latest release: Shmew/Feliz.MaterialUI#59.

Looking into the error it seems that a const is resolved as a function. In the error example the following constant is called:

/* tslint:disable max-line-length */
/**
 * ![indigo 50](https://material-ui.com/static/colors-preview/indigo-50-24x24.svg) ![indigo 100](https://material-ui.com/static/colors-preview/indigo-100-24x24.svg) ![indigo 200](https://material-ui.com/static/colors-preview/indigo-200-24x24.svg) ![indigo 300](https://material-ui.com/static/colors-preview/indigo-300-24x24.svg) ![indigo 400](https://material-ui.com/static/colors-preview/indigo-400-24x24.svg) ![indigo 500](https://material-ui.com/static/colors-preview/indigo-500-24x24.svg) ![indigo 600](https://material-ui.com/static/colors-preview/indigo-600-24x24.svg) ![indigo 700](https://material-ui.com/static/colors-preview/indigo-700-24x24.svg) ![indigo 800](https://material-ui.com/static/colors-preview/indigo-800-24x24.svg) ![indigo 900](https://material-ui.com/static/colors-preview/indigo-900-24x24.svg) ![indigo A100](https://material-ui.com/static/colors-preview/indigo-A100-24x24.svg) ![indigo A200](https://material-ui.com/static/colors-preview/indigo-A200-24x24.svg) ![indigo A400](https://material-ui.com/static/colors-preview/indigo-A400-24x24.svg) ![indigo A700](https://material-ui.com/static/colors-preview/indigo-A700-24x24.svg)
 */
declare const indigo: {
  /**
   * Preview: ![indigo 50](https://material-ui.com/static/colors-preview/indigo-50-24x24.svg)
   */
  50: '#e8eaf6';

While in the generated javascript file it is called as:

mkStyle("color", indigo()["50"])

This is the line of F# code that causes the above problem:

                        prop.style [ 
                            style.padding 10
                            style.color Colors.indigo.``50`` 
                        ]

So, where does this problem originate? It seems the errors mentioned in Feliz.MaterialUI are not specific to that lib, I would guess it is either Feliz or the Nagareyama?

@alfonsogarciacaro
Copy link
Contributor Author

Ah, I wasn't aware of this issue. Thanks for pointing it out, I will check.

@halcwb
Copy link

halcwb commented Dec 22, 2020

Ah, I wasn't aware of this issue. Thanks for pointing it out, I will check.

Great! Everything works. I tried it just know following the above recipe.

Just two caveats:

When optioning for a different outDir, the order matters:

dotnet fable watch --outDir build --run webpack-dev-server

works

dotnet fable watch --run webpack-dev-server --outDir build

doesn't work. the outDir isn't used.

Also, when using the outDir, the generated files just have the js extension. So you need to change you're webpack entry point accordingly.

@alfonsogarciacaro
Copy link
Contributor Author

Great, thanks for confirming it works @halcwb!

About the --run option, everything after it is passed to the external command. It's true it can be confusing, we have to document it better. Likewise for the extension, you can make it explicit with the --extension argument, but the default is different depending on whether you also use --outDir or not. The reasoning is here but again we need to document it better.

Booksbaum added a commit to Booksbaum/ionide-vscode-helpers that referenced this pull request Dec 30, 2020
based on MangelMaxime/fulma-demo#43

build script isn't updated yet.
Use ` dotnet fable .\src\Fable.Ionide.VSCode.Helpers.fsproj --outDir release`
Krzysztof-Cieslak pushed a commit to ionide/ionide-vscode-helpers that referenced this pull request Dec 30, 2020
based on MangelMaxime/fulma-demo#43

build script isn't updated yet.
Use ` dotnet fable .\src\Fable.Ionide.VSCode.Helpers.fsproj --outDir release`
@alfonsogarciacaro
Copy link
Contributor Author

alfonsogarciacaro commented Jan 8, 2021

Source maps are back in Fable 3.1! Check this commit, these are the instructions to enable them:

  1. Add *.fs.js.map to your .gitignore if necessary
  2. Update to Fable 3.1 or higher: dotnet tool update fable
  3. To consume the source maps with Webpack you need to install the source-map-loader npm package. At least with Webpack 4, v2.0 gave me problems so please stick to v1.1.3: npm i --save-dev [email protected] or yarn add --dev [email protected]
  4. Add the following to your webpack config
  5. In the fable command pass the --sourceMaps or -s flag: dotnet fable watch src -s --run webpack-dev-server
  6. Profit!

@MangelMaxime
Copy link
Owner

MangelMaxime commented Jan 19, 2021

@alfonsogarciacaro I am really impressed by all the hard work you put in Fable 3. 👍 👍 👍 👍 👍

I am finally becoming comfortable with Fable 3 as I am starting to use it at work and on some projects. So I should soon be able to accept this PR and upgrade the tool chain to webpack5 too.

It seems like I have it working on my projects.

@MangelMaxime
Copy link
Owner

To consume the source maps with Webpack you need to install the source-map-loader npm package. At least with Webpack 4, v2.0 gave me problems so please stick to v1.1.3: npm i --save-dev [email protected] or yarn add --dev [email protected]

This is normal:

Release note of source-map-loader

image

I am using version 2.0.0 with Webpack 5 on my project without problem.

@kerams
Copy link

kerams commented Jan 19, 2021

@MangelMaxime, did you manage to make hot reload work in Webpack 5 in the end? I'm using a beta version of webpack-dev-server but no dice. Sass hot reload works though...

@MangelMaxime
Copy link
Owner

@kerams From memory, I think I had it working on https://github.com/MangelMaxime/Herebris/ repo.

The repo is open sourced but not at all ready for people to easily use it. I am working on the Express bindinds right now and I am kind of stuck in a binding hell ^^

Here is the webpack config file: https://github.com/MangelMaxime/Herebris/blob/5edb8eb75e0e1f5e7684aea9ddb39dc89f2a6d26/Client/webpack.config.js

It contains @pmmmwh/react-refresh-webpack-plugin but I think it can be removed as fast-refresh doesn't play well with "real/strict" Elmish application.

Even if you try to use Feliz.UseElmish it doesn't work for child because the reference on dispatch doesn't seems to be updated correctly.

Booksbaum added a commit to Booksbaum/ts2fable that referenced this pull request Mar 6, 2021
SourceMaps need `source-map-loader` to work with webpack.
See: MangelMaxime/fulma-demo#43 (comment)
Booksbaum added a commit to fable-compiler/ts2fable that referenced this pull request Mar 6, 2021
SourceMaps need `source-map-loader` to work with webpack.
See: MangelMaxime/fulma-demo#43 (comment)
@MangelMaxime MangelMaxime merged commit e7339b8 into MangelMaxime:master Mar 10, 2021
@MangelMaxime
Copy link
Owner

Thank you @alfonsogarciacaro and everybody who contributed to this PR

mlaily added a commit to mlaily/lastfmstats that referenced this pull request Jun 27, 2021
MaxWilson pushed a commit to MaxWilson/SourceMapRepro that referenced this pull request Jun 9, 2022
MaxWilson pushed a commit to MaxWilson/SourceMapRepro that referenced this pull request Jun 9, 2022
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.

8 participants