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

enable live debugging #8

Merged
merged 6 commits into from
Feb 9, 2018
Merged

Conversation

humhei
Copy link
Contributor

@humhei humhei commented Feb 9, 2018

#7

@humhei humhei changed the title Enable live debugging enable live debugging Feb 9, 2018
@humhei
Copy link
Contributor Author

humhei commented Feb 9, 2018

There is a bug
After build , bundle.js will existed in ./public directory
In this case, breakpoint will not be caught.
So i added webpack.config.dev.js and webpack.config.prod.js just as create-react-app done,
the build command will output to "./build" directroy

@alfonsogarciacaro alfonsogarciacaro merged commit 0f39b10 into fable-compiler:master Feb 9, 2018
@alfonsogarciacaro
Copy link
Member

alfonsogarciacaro commented Feb 9, 2018

Great work @humhei, thanks a lot for this! The template needed some love 😉

I also took the chance to "hide" the Webpack config files in the tools dir so they don't scare newcomers. This is a similar though slightly different approach to what we discussed in fable-scripts. Could you (and maybe also @MangelMaxime @whitetigle) have a look and tell me what you think before publishing a new version of the template?

Please check the README to see the changes (the last section "Project Structure" hasn't really changed so you can skip it). Some notes:

  • I'm only using yarn to make things simpler for users
  • yarn automatically calls dotnet restore
  • Fable (and other CLI tools if necessary) are also moved to tools dir and called automatically by yarn start/build
  • To avoid the need to worry about webpack config files I've moved Fable config details (for now, only the entry file) to a fable section in package.json
  • I haven't added SASS compilation to the template yet, maybe we should do it to share configuration with the Fulma template?

@MangelMaxime
Copy link
Member

Personally, I think by doing this we just delay the moment when users will need to underscore paket, yarn, webpack, etc.

But for the simple template, it make sense so I am ok with that.

@humhei
Copy link
Contributor Author

humhei commented Feb 9, 2018

I tried to run task
but interruped with below message
Executing task: C:\Program Files (x86)\Yarn\bin\yarn start <

Update:
Only add this line in lanuch.json will get it worked
"isShellCommand": true,

@alfonsogarciacaro
Copy link
Member

@humhei Do you have yarn installed? Does it change if you replace yarn with npm in this line?

@MangelMaxime Yes, for that we still need the tutorial series 😉 But for the template I think most of users want to start as soon as possible and many get confused if there are extra "unusual" steps (I was helping a user yesterday and after trying several things the problem was they were not running dotnet fable from src dir).

@humhei
Copy link
Contributor Author

humhei commented Feb 10, 2018

@alfonsogarciacaro I like the new files structure tree 👍
I have three suggestions below

  • How about place index.html in public dir just as create-react-app done?
    I Think this is more common
  • Update tasks.json to version 2.0. As it get worked for me
{
    // See https://go.microsoft.com/fwlink/?LinkId=733558
    // for the documentation about the tasks.json format
    "version": "2.0.0",
    "tasks": [
        {
            "command": "yarn",
            "args": ["start"],
            "type": "shell",
            "label": "Start",
            "group": "build",
            "isBackground": true,
            "problemMatcher": {
                "fileLocation": "absolute",
                "background": {
                    "activeOnStart": true,
                    "beginsPattern":{
                        "regexp": "webpack: Compiling"
                    },
                    "endsPattern":{
                        "regexp": "webpack: (Compiled successfully|Failed to compile)"
                    }
                },
                "pattern": {
                    "regexp": "^(.*)\\((\\d+),(\\d+)\\): \\((\\d+),(\\d+)\\) (warning|error) FABLE: (.*)$",
                    "file": 1,
                    "line": 2,
                    "column": 3,
                    "endLine": 4,
                    "endColumn": 5,
                    "severity": 6,
                    "message": 7
                }
            }
        }
    ]
}
  • Use FAKE CLI by default and place fsx files in tools dir
    "build": "cd tools && dotnet fable webpack -- -p --config webpack.config.prod.js",
    "start": "cd tools && dotnet fable webpack-dev-server -- --config webpack.config.dev.js",

Above code is bad to read
I more like seeing below scripts

    "build": "fake run tools/build.fsx",
    "start": "fake run tools/start.fsx",

@humhei
Copy link
Contributor Author

humhei commented Feb 10, 2018

I also took a chance to add FAKE in workflow #9
It works well for me besides the console color of webpack is all the yellow

@humhei humhei mentioned this pull request Feb 10, 2018
@humhei
Copy link
Contributor Author

humhei commented Feb 11, 2018

The last commit fixed the yellow output,
Webpack console color will be normal

@alfonsogarciacaro
Copy link
Member

alfonsogarciacaro commented Feb 13, 2018

Thanks for the tip about VS Code tasks version @humhei! I upgraded it. I saw you closed your PR, are you planning to send it again? Adding FAKE to the basic template is a bit controversial, and has been discussed some times before. In principle, I'd like to avoid it because it's not strictly necessary and it adds some cognitive overhead specially to users coming from JS, who already need to learn about F#, dotnet SDK, Paket...

Should we publish the template as it is now so users can give feedback?

@humhei
Copy link
Contributor Author

humhei commented Feb 14, 2018

I feel the same

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

3 participants