choojs / bankai

:station: - friendly web compiler
Apache License 2.0
1.09k stars 102 forks source link

Bankai v9 beta feedback #246

Open yoshuawuyts opened 7 years ago

yoshuawuyts commented 7 years ago

Feedback on the v9 beta thread. Post below if things don't line up alright for you. Thanks!

s3ththompson commented 7 years ago

Not feedback so much as an idea... I've been thinking about the convo we (@YerkoPalma @yoshuawuyts) were having about how to server-side render dynamic routes.

Seems like you could avoid complexity in Bankai if you just wrote your app slightly differently:

app.route('/posts/:id', require('./views/post'))

to

for (let post of posts) {
    app.route(`/posts/${post.id}`, require('./views/post'))
}

Then I guess the only difficulty would be figuring out how to get the route params into the view without manually reading them from window.location...

s3ththompson commented 7 years ago

Just added issue #247 and PR #248

s3ththompson commented 7 years ago

Ran into issue #249 as well (which you may already have on your radar per this comment: https://github.com/choojs/bankai/pull/241#issuecomment-332536916)

s3ththompson commented 7 years ago

Really, going ham on testing this new pipeline :)

Opened yoshuawuyts/inline-critical-css#1 which seems like a common use case

s3ththompson commented 7 years ago

250 is the biggest issue for me right now...

jondashkyle commented 7 years ago

Biggest thing for me right now (which I know is on the radar) is some way of nicely playing with browserify transforms static assets (aka brfs, enoki, etc…) These currently work but really slow down the pipeline!

yoshuawuyts commented 7 years ago

Then I guess the only difficulty would be figuring out how to get the route params into the view without manually reading them from window.location...

@s3ththompson perhaps state.href? :tada:

cjhowedev commented 7 years ago
for (let post of posts) {
    app.route(`/posts/${post.id}`, require('./views/post'))
}

@s3ththompson The problem with this is the situation where new posts are added. You'd have to add a new route every time a new object is created. I'm not sure it's the best idea to push this complexity onto the user.

s3ththompson commented 7 years ago

@cjhowe7 in this case, I'm thinking of server-side rendering is an optimization the developer adds with some knowledge ahead-of-time of which routes will be most popular (or a feature to help develop completely static websites). You could always fall back to a dynamic route:

for (let post of popularPosts) {
    app.route(`/posts/${post.id}`, require('./views/post'))
}
app.route(`/posts/:id`, require('.views/post'))
jbergstroem commented 7 years ago

While passing -d to start (./bin.js start -d example) the experience is not great. It draws the progress-bars then exits with no message. It doesn't really give the user a clue what went right or wrong.

cjhowedev commented 7 years ago

I figured out my issue with bankai. I was passing the directory that contained index.js instead of the path to index.js itself. My bad!

YerkoPalma commented 7 years ago

@yoshuawuyts is there a chance to add support for custom host, different from localhost, again? If it is ok I can send a PR for a --host option. Besides of that, the dynamic route is still a critical issue for me, dynamic routes are much more important than SSR, IMO. Is an option to add a litle server.js on build that use hyperstram to resolve ssr on dynamic routes?

yoshuawuyts commented 7 years ago

@YerkoPalma is there a way to detect the special env you're running in that doesn't have localhost? Alternatively we could also serve up the internal IP address instead of localhost; but yeah — ideally we could evade adding options flags (:

jbergstroem commented 7 years ago

@yoshuawuyts the obvious other host other than localhost would be 0.0.0.0 I guess.

YerkoPalma commented 7 years ago

@yoshuawuyts it is a remote Google App Engine that do has localhost, but that host isn't public availaible, so to access that container instance I have to use 0.0.0.0 as @jbergstroem said. It is not a common case I guess, but I think any one developing on the cloud could spot on this, maybe haha

zigomir commented 6 years ago

It's not beta anymore, but I'll write here anyway because I'm not sure if issue is even bankai or create-choo-app. So I updated all packages from create-choo-app and I'm using bankai 9.0.1 and after I run npm run build and serve files from dist, there are two bundle.css requests.

Adding screenshot here

choo bankai 9