Yew CLI development PR by philip-peterson · Pull Request #1419 · yewstack/yew

philip-peterson · 2020-07-19T02:30:07Z

Description

This is a PR for tracking the development of Yew CLI

teymour-aldridge · 2020-07-19T10:16:59Z

@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8"?>


I think editor setup shouldn't be commited to the repo. Would you mind adding this to the .gitignore?

teymour-aldridge · 2020-07-19T10:17:53Z

+rayon = "1.3.1"
+lazy_static = "1.4.0"
+log = "0.4.11"
+clap = "2.33.1"


I think these can be a little less specific (I know IntelliJ tends to fill in with the most specific possible) because of how semantic versioning works. e.g.

Suggested change

clap = "2.33.1"

clap = "2.33"

teymour-aldridge · 2020-07-19T10:19:50Z

+
+use log::{error, info, warn};
+
+// Usages:


These would be better as doc comments attached to the main function.

/// content ... fn main() { ... }

teymour-aldridge · 2020-07-19T10:20:14Z

+                .about("compiles a Yew application")
+                .arg(
+                    Arg::with_name("run")
+                        .help("Start a webserver for the built project and open it in a browser window")


Suggested change

.help("Start a webserver for the built project and open it in a browser window")

.help("Start a web server for the built project and open it in a browser window")

teymour-aldridge · 2020-07-19T10:21:25Z

+                        .short("r")
+                )
+                .arg(
+                    Arg::with_name("release")


I think this could be better handled by taking everything after -- and passing it to cargo (like how cargo allows you to pass arguments to rustc, e.g. cargo test -- --nocapture where --nocapture is passed to rustc).

philip-peterson · 2020-07-19T10:40:06Z

@teymour-aldridge All valid comments. It should be noted though, this is a draft PR and still very WIP so we probably won’t really be ready for a review for a number of iterations. :-)

teymour-aldridge · 2020-07-19T10:41:46Z

@philip-peterson I realise that :P – I'm just making one or two suggestions here for future development before the work is done rather than afterwards (to minimise wasted effort).
Give me a shout if there's anything you'd like any help on (I'll do my best to try and assist!)

philip-peterson · 2020-07-19T10:51:58Z

Fair enough! It is helpful feedback to have, will give another shout when things are farther along. Thanks for the initial feedback 🙏

# Conflicts: # yew-cli/Cargo.toml # yew-cli/src/main.rs

siku2 · 2020-08-24T14:47:50Z

How far along are we with this? I'm seeing some rather surprising things in here like yew-config. Looks promising, but what's the goal there?

Now that we have #1504 for the internal maintenance tasks we no longer need this to take care of them.
I think that's a good thing because this has shaped more into a tool for developers than maintainers and having it be both at the same time comes at a cost for both parties.

The CLI stands completely on its own now so I would like to give it its own repository in the yewstack organisation.
The reason for this is that the CLI isn't directly coupled with the library and should probably have an independent release cycle. It also allows it to make better use of various GitHub features which could get quite messy if it was part of the monorepo.

What do you think, @jstarry?

philip-peterson · 2020-08-24T23:01:58Z

The goal of yew-config was to solve the following problem: we want users to be able to run all the examples at once, and that means compiling them and starting a web server for each. However we can’t specify whether each example is wasm-bindgen or wasm-pack on a per-example basis, as that would require a flag for each example, negating the convenience of starting all examples at one time. (We can currently only specify the build scheme for all projects across the board, however, some examples use wasm-pack and others don’t.) Because of this, we really wanted a per-project config standard so that each project could specify whether it wants wasm-pack or not, as well as maybe some settings like route structures for SSG/SSR down the line.

Yew-config allows the user to specify a configuration on each project, using normal Rust typing mechanisms instead of ad-hoc unspecified dynamic config files a la Webpack. The configs will be read by the yew-cli tool before starting up the web servers for the examples. I do think it’s a tad unrelated and can be merged as its own PR first, but it’s still a requirement for that feature of yew-cli.

siku2 · 2020-08-25T12:12:57Z

I love that it's possible to run all examples at once but I don't think that's something we were really missing :)

I also really like the idea of having a config for Yew projects but to be brutally honest I don't like the idea of having the configuration in the actual Rust code which even adds an additional dependency to the project. I'm willing to change my mind on this if you can point me to a project that uses this approach successfully but right now I'm very much in favour of a simple yew.toml file.

Why are we distinguishing between wasm-bindgen and wasm-pack? wasm-pack build is just a shortcut for cargo build -> wasm-bindgen -> wasm-opt (the main benefit being that it can automatically download these things).
There's no need to switch between the two because they are basically the same thing.

I think yew-cli should behave like a fork of wasm-pack but geared towards applications instead of libraries. Most of the functionality we need is already in wasm-pack, we just need to add things like the dev server.

philip-peterson · 2020-08-26T22:17:47Z

Wasm-pack requires that there be a function annotation #[wasm_bindgen(start)] on some function and might also require a wasm-bindgen dependency in Cargo -- see the differences between examples/minimal and examples/minimal_wp. It would be nice if we could eliminate one of the schemes.

As for yew-config, that's pretty disappointing to hear .. I would prefer not to be the person to introduce yet another untyped, schema-less config to the web development ecosystem. Would sooner vote to just remove the ability to run all examples at once. I really think we'll need yew-config at some point soon, especially if we want to run the yew-router examples which each have their own independent lists of routes that correspond to the index.html. We can kick the can down the road for now by not including it but I expect it will come up eventually.

siku2 · 2020-08-26T22:59:50Z

Wasm-pack requires that there be a function annotation #[wasm_bindgen(start)] on some function and might also require a wasm-bindgen dependency in Cargo -- see the differences between examples/minimal and examples/minimal_wp.

There are a few differences I didn't mention because they are mostly surface-level and only exist because wasm-pack is designed for libraries.
wasm-pack doesn't technically need a function with the #[wasm_bindgen(start)] annotation, it just doesn't generate an entry point because it uses cargo build --lib to generate the WASM binary (it also requires the crate to be cdylib for this reason).
If wasm-pack ran cargo build instead, these differences would disappear.
Anyway, all I was trying to say is that they fundamentally do the same thing only that wasm-pack operates at a higher level and is designed for libraries which isn't what we need for Yew apps.

It would be nice if we could eliminate one of the schemes.

Yes, I absolutely agree. wasm-pack was never really meant to be used for Yew projects but it's flexible enough to make it work.
I believe we should only use wasm-bindgen in the end.
wasm-pack already has a lot of the functionality we want (automatic downloads, wasm-opt, testing) which is why I'm suggesting we basically copy it but rip out all the parts that apply to libraries and make it work for applications.

philip-peterson · 2020-08-27T00:19:20Z

I'm a bit skeptical of that plan, wouldn't it be better to have wasm-pack own support for this? wasm-pack looks like a pretty large surface area to maintain, and I'm not really sure what we would gain from forking it.

siku2 · 2020-08-27T17:08:48Z

I'm a bit skeptical of that plan, wouldn't it be better to have wasm-pack own support for this?

That would make some sense, but I doubt the maintainers of wasm-pack want to add this functionality. My understanding of wasm-pack's scope is that it's meant to integrate Rust into the Javascript ecosystem and everything it does is to accommodate that goal.
In order for wasm-pack to really work for us it would need things like a wasm-pack start command and the option to build binary targets instead of libraries. Both of those don't really make sense for a tool designed to publish Rust WASM libraries to NPM.

wasm-pack looks like a pretty large surface area to maintain, and I'm not really sure what we would gain from forking it.

Well, what's the alternative then? If we use wasm-bindgen-cli directly we'll just end up re-implementing a lot of things we could've taken from wasm-pack.
I agree that wasm-pack isn't exactly tiny but it has a lot of features I wouldn't want to miss out on.

philip-peterson · 2020-08-27T18:23:26Z

They would probably rather support the feature than have us fork their library and split the ecosystem. We should at least ask IMO. But, also I would posit that we don’t exactly need to have this — we already invoke wasm-pack with success, it’s just not ideal because of the annotation and cargo dependency. MVP for yew-cli at least doesn’t require a wasm-pack fork for now.

siku2 · 2020-08-27T18:56:46Z

I wouldn't really call it "splitting the ecosystem" if the tools serve two distinct purposes but I admit that it doesn't hurt to ask first.

MVP for yew-cli at least doesn’t require a wasm-pack fork for now.

Just to make this clear: I'm not advocating for creating a fork of wasm-pack to be used by yew-cli. I'm saying yew-cli should be the fork of wasm-pack.

Anyway, I think it's fine to have yew-cli call out to wasm-pack for the test command for now but I would rather we didn't use it for the build command because then we'd either have to drop support for raw wasm-bindgen or have it conform to the standards set by wasm-pack otherwise we would end up splitting the ecosystem.

philip-peterson · 2020-08-27T19:56:30Z

I’m confused. Why would supporting wasm-pack require dropping support for wasm-bindgen? We already support both.

jstarry · 2020-09-20T07:29:09Z

I’m confused. Why would supporting wasm-pack require dropping support for wasm-bindgen? > We already support both.

@philip-peterson Because examples that are setup as binaries (rather than libraries) and use wasm-bindgen cannot be built with wasm-pack build because it only builds libraries.

Personally, I think that trunk is the way forward here. Of course there could be room for an alternative tool to trunk and it could even be called yew-cli if it's intended to be Yew specific.

I do agree with @siku2 that this PR is not the right place for that exploration. I would prefer if this was moved to a separate non-yewstack repo (like many other yew libraries) and developed independently. In the future, I expect some of the independent libraries will become first class members of the yew monorepo. The best the Yew maintainers can do is help the community discover and collaborate on these libraries while they are still in their infancy.

I'm going to close this PR due to this being the wrong place for this type of exploration. Please feel free to open a PR that would add links to a new yew-cli repo and we can add channels in the Yew Discord if needed

initial commit on the yew cli

36794c8

philip-peterson closed this Jul 19, 2020

philip-peterson reopened this Jul 19, 2020

philip-peterson marked this pull request as draft July 19, 2020 02:32

philip-peterson added 4 commits July 18, 2020 22:01

use clap

14e213a

accept projects

1303726

format

083f8b9

cleanup

f9739f9

teymour-aldridge reviewed Jul 19, 2020

View reviewed changes

cramt added 3 commits July 19, 2020 13:32

accepting projects now works properly, executing wasm-pack also works

3adcb2a

cleanup + making of html file if there isnt one already

59979c2

added deletion of .gitignore, we should probably make that a flag

6296bb7

siku2 linked an issue Jul 19, 2020 that may be closed by this pull request

Proposal: Replace bash scripts with a Rust CLI #1418

Closed

implemented tokio runtime and the run option

383b4be

siku2 mentioned this pull request Jul 20, 2020

Update main yew examples (remove static dir) #1417

Merged

2 tasks

philip-peterson mentioned this pull request Jul 24, 2020

Proposal: Replace bash scripts with a Rust CLI #1418

Closed

philip-peterson added 11 commits July 24, 2020 22:24

rename cli to yew-cli

459d373

remove .idea

ef23f33

Add .gitignore for yew-cli

3e5e584

Remove IDE file

b120aad

error handling

c56a879

organize subcommands

a8a8ab1

Handle flags, do some cleanup

cedbb1a

reduce warnings

9ba7d2d

cargo fmt

4a1a746

remove unused imports

cc5bb90

Logging, try to open web browser (doesn't work yet)

7c5d05e

philip-peterson and others added 17 commits August 1, 2020 21:42

separate execution logic

c9dbc3c

new subcommand is now a thing, creates a new yew project

4d2a5c4

Merge remote-tracking branch 'origin/yew-cli' into yew-cli

d18851f

# Conflicts: # yew-cli/Cargo.toml # yew-cli/src/main.rs

merged

7612840

removed unnesecary file

15f8a64

fixed WASM32_TARGET_NAME compile errors

7000672

add back windows canonicalization

20fcdcc

wip analyzer

50557b8

fix up declaring configs

5458afa

auto-type the config

aa6406d

format

9d7566a

wip declaration of a config

e066c26

cleanup

96b0e85

Update error message

1a43e90

Tweak macro name

51c2d0a

semantics for macro invocation

314e4e2

minor semantic fix

96531b7

siku2 removed a link to an issue Aug 24, 2020

Proposal: Replace bash scripts with a Rust CLI #1418

Closed

jstarry closed this Sep 20, 2020

	.help("Start a webserver for the built project and open it in a browser window")
	.help("Start a web server for the built project and open it in a browser window")

Uh oh!

Conversation

philip-peterson commented Jul 19, 2020

Description

Uh oh!

teymour-aldridge Jul 19, 2020

Choose a reason for hiding this comment

Uh oh!

teymour-aldridge Jul 19, 2020

Choose a reason for hiding this comment

Uh oh!

teymour-aldridge Jul 19, 2020

Choose a reason for hiding this comment

Uh oh!

teymour-aldridge Jul 19, 2020

Choose a reason for hiding this comment

Uh oh!

teymour-aldridge Jul 19, 2020

Choose a reason for hiding this comment

Uh oh!

philip-peterson commented Jul 19, 2020

Uh oh!

teymour-aldridge commented Jul 19, 2020

Uh oh!

philip-peterson commented Jul 19, 2020

Uh oh!

siku2 commented Aug 24, 2020

Uh oh!

philip-peterson commented Aug 24, 2020

Uh oh!

siku2 commented Aug 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

philip-peterson commented Aug 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

siku2 commented Aug 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

philip-peterson commented Aug 27, 2020

Uh oh!

siku2 commented Aug 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

philip-peterson commented Aug 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

siku2 commented Aug 27, 2020

Uh oh!

philip-peterson commented Aug 27, 2020

Uh oh!

jstarry commented Sep 20, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

siku2 commented Aug 25, 2020 •

edited

Loading

philip-peterson commented Aug 26, 2020 •

edited

Loading

siku2 commented Aug 26, 2020 •

edited

Loading

siku2 commented Aug 27, 2020 •

edited

Loading

philip-peterson commented Aug 27, 2020 •

edited

Loading