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

Get a few example apps in place #8

Closed
2 tasks done
thedodd opened this issue Aug 25, 2020 · 8 comments
Closed
2 tasks done

Get a few example apps in place #8

thedodd opened this issue Aug 25, 2020 · 8 comments
Labels
good first issue Good for newcomers help wanted Extra attention is needed ready Ready to be implemented

Comments

@thedodd
Copy link
Member

thedodd commented Aug 25, 2020

  • add a Yew example.
  • add a Seed example.
@siku2
Copy link
Contributor

siku2 commented Sep 15, 2020

As for Yew, I'm currently moving our entire collection of examples over to trunk. Of course these examples aren't going to use the entire feature set of trunk just yet (because prior to trunk it was simply too hard to set these things up for each example) but hopefully that's going to change in the future.

See: yewstack/yew#1559

@thedodd
Copy link
Member Author

thedodd commented Sep 15, 2020

@siku2 that's awesome! I'm super stoked to hear that. Please let me know if there is anything I can do to help on that front.

@siku2
Copy link
Contributor

siku2 commented Sep 15, 2020

So far everything is going great but there is a pretty significant feature I'd like to discuss (relating to web workers). I'll open an issue when I get to that though.

One small thing I would like to ask is whether you'd accept a CLI flag to specify the package to be built (akin to cargo run --package).
That way we could say

Use trunk serve -p todomvc to run the example

instead of cd todomvc && trunk serve which isn't as portable.

@thedodd
Copy link
Member Author

thedodd commented Sep 15, 2020

@siku2 one option that is current available is to just specify trunk serve --manifest-path todomvc/Cargo.toml mirroring cargo's --manifest-path option.

I considered adding support for -p/--project for mirroring cargo's workspace projects pattern. I would be down for that. I had also considered supporting the --example ... but I have some reservations on that one.

Thoughts?

@siku2
Copy link
Contributor

siku2 commented Sep 15, 2020

@thedodd I tried using --manifest-path but then one also needs to set <target> to <path>/index.html which makes it really verbose (trunk --manifest-path todomvc/Cargo.toml serve -- todomvc/index.html). The dist directory is also in the wrong location unless overwritten.

I don't hate the idea of --example because cargo run --example doesn't work for these projects but IMO that is a separate issue.
I think -p/--project (though perhaps it should be --package to stay consistent with cargo?) is the way to go.

@thedodd
Copy link
Member Author

thedodd commented Sep 15, 2020

@siku2 ah, with the --manifest-path that makes sense. Do you think the location of the dist dir is a bug? Sounds like it might be. IIRC, I'm just using the parent dir of the manifest's target dir as the location for the dist dir (which works for workspaces as well); however, I could be wrong on that. I may have neglected to do that last bit.

though perhaps it should be --package to stay consistent with cargo?

Yes, you are correct. That's my bad.

So with -p/--package, are you thinking that you would like for the behavior to be such that Trunk will essentially treat the value of -p as though it were the CWD for resolving the location of dist, index.html &c?

@siku2
Copy link
Contributor

siku2 commented Sep 15, 2020

Do you think the location of the dist dir is a bug?

Glancing at the implementation (opts.dist.unwrap_or_else(|| "dist".into())) it doesn't do anything fancy like using the target's parent directory.
If anything I think it should be relative to --manifest-path because users might have a setup where the HTML file is placed in a subfolder like static/index.html.


So with -p/--package, are you thinking that you would like for the behavior to be such that Trunk will essentially treat the value of -p as though it were the CWD for resolving the location of dist, index.html &c?

Yeah, that's all I would need for now. There is a bit more to cargo's --package flag though, for instance, it can be specified multiple times.
I would have to take a closer look at Cargo's code to tell how it's implemented.

@thedodd thedodd changed the title Get a few example apps in place & add gif/screenshots to readme Get a few example apps in place Oct 2, 2020
@thedodd thedodd added good first issue Good for newcomers help wanted Extra attention is needed ready Ready to be implemented labels Oct 2, 2020
@thedodd
Copy link
Member Author

thedodd commented Jul 28, 2021

Always room to add more examples, but I would say that we can pretty safely call this issue closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed ready Ready to be implemented
Projects
None yet
Development

No branches or pull requests

2 participants