ember-learn / ember-cli-addon-docs

Easy, beautiful docs for your OSS Ember addons
https://ember-learn.github.io/ember-cli-addon-docs
MIT License
176 stars 143 forks source link

Integrate Prember/FastBoot #93

Open elwayman02 opened 6 years ago

elwayman02 commented 6 years ago

Since demo sites are mostly static, we could leverage Prember to make addon demos load really fast and avoid authors having to figure it out on their own.

scalvert commented 5 years ago

We're looking into this too, as it's a requirement for us to publish a static site to allow search indexing with our broader internal systems. We'll let you know what we come up with.

lolmaus commented 4 years ago

The readme of my addon has gone way out of control, it's 72 KiB long. 🙈

I've tried migrating it to Addon Docs. The result looks absolutely awesome, but I was discouraged to find out that my documentation is no longer indexable.

This is a huge downside.

RobbieTheWagner commented 4 years ago

Perhaps I am wrong, but couldn't people add prember and fastboot as dev deps and point them at the dummy app?

lolmaus commented 4 years ago

@rwwagner90 It should be available out of the box, preconfigured. All the FastBoot business, shoeboxing, handling network requests, whitelisting libraries, etc.

RobbieTheWagner commented 4 years ago

@lolmaus should the documentation need all those features? Seems like overkill to me.

lolmaus commented 4 years ago

@rwwagner90 The documentation surely needs to be indexed.

As for shoeboxing and such, I don't know! I have a very vague idea how ember-cli-addon-docs is designed under the hood.

RobbieTheWagner commented 4 years ago

@lolmaus Indexed, sure. All we need for that is prember. I don't know of documentation that uses Ember data, requires network requests, needs to whitelist libraries etc.

With this in mind, I think the consumers of ember-cli-addon-docs are free to add prember to the dummy site as they wish. Do you see any reason they could not?

lolmaus commented 4 years ago

Well, they could also configure ember-cli-deploy on their own, deal with rootURL and version numbers on their own, figure out YUIDoc integration on their own...

But for some reason you've chosen to bundle all this pre-configured. Probably because you want users to focus on writing documentation and not spending time researching how to deal with all that.

Without prember, ember-cli-addon-docs surely feel incomplete. Is there a reason why prember should not come preconfigured with ember-cli-addon-docs?

lolmaus commented 4 years ago

I think the consumers of ember-cli-addon-docs are free to add prember to the dummy site as they wish. Do you see any reason they could not?

For example, I have no idea how to address this: https://github.com/ember-learn/ember-cli-addon-docs/issues/327

RobbieTheWagner commented 4 years ago

Well, they could also configure ember-cli-deploy on their own, deal with rootURL and version numbers on their own, figure out YUIDoc integration on their own...

I don't think that's a fair example. Then there would be no need for this addon.

327 would have to be dealt with, yes. I did not think about that. Aside from that though, what I am trying to say here is prember is a simple install away. I would advocate for documenting how to use it, rather than forcibly bundling.

lolmaus commented 4 years ago

I don't think that's a fair example. Then there would be no need for this addon.

That was my point. Any addon is just a means to save you some effort.

There is no reason for addon documentation to be unavailable as static HTML. Without static HTML, ember-cli-addon-docs is surely incomplete. Pages are slower to render than they could be, and documentation is not indexed by search engines.


prember is a simple install away

What's the problem with having it out of the box?


I would advocate for documenting how to use it, rather than forcibly bundling.

Well, this is definitely better than nothing, but I still see no point in not including it.

RobbieTheWagner commented 4 years ago

Well, this is definitely better than nothing, but I still see no point in not including it.

It's not a default thing for Ember apps. Would you say it should be the default for all Ember apps?

lolmaus commented 4 years ago

Uhm, no. But it should definitely be the default for addon docs.

Do you imply that prember makes little sense for interactive addon demos typically served from dummy apps?

In that case, the out-of-the box configuration of ember-cli-addon-docs could enable prember only for the docs pages and omit the rest of the dummy app. The rest of the dummy app could be opt-in.

IMO, that's another argument for bundling and preconfiguring prember with ember-cli-addon-docs. 😏

RobbieTheWagner commented 4 years ago

@lolmaus I think I am mainly saying it should be up to the user. If prember is the default, and you really wanted a normal Ember app, you would need to find a way to disable it. I'm a big fan of prember, and I totally agree with you that it should be possible to use for addon docs, but I think it should be up to the user to opt in to, or if we made it the default, there needs to be an easy opt-out.

lolmaus commented 4 years ago

@rwwagner90 Can you think of a case where an addon author would want to disable prember for the documentation pages?

RobbieTheWagner commented 4 years ago

Off the top of my head, no, but I think making it opt in / opt out should be fine and support all cases. @samselikoff @dfreeman @pzuraq what do you guys think about making prember the default? Should it be default with an opt out or not enabled by default, with an opt in?

elwayman02 commented 4 years ago

I'm in favor of making it a default with easy opt out.

samselikoff commented 4 years ago

No reason for it not to be included, simply hasn't been done yet! We just need someone motivated who's willing to contribute.

AddonDocs is a great use-case for Prember out of the box since all the data is known at build time.

Note: Prember apps are still "normal Ember apps" so all client-side interaction will still work.

I do believe https://github.com/ember-learn/ember-cli-addon-docs/issues/327 was the last time I attempted this, and indeed ran into some issues with the way we handle deploys + what Prember expects. If someone wants to work on this, that issue would be the right place to start 👍

Also, a great way to get a big change like this merged into master is to follow the example of @ef4's work in https://github.com/ember-learn/ember-cli-addon-docs/pull/409. The feature was tested against Ember Animated so there were plenty of edge cases to check. (No automated tests made it in alongside the PR, but still, having it run against a complex site like Ember Animated was reassuring.) So, I would suggest to whomever wants to work on this, to start by pointing an addon to a local branch of AddonDocs and ensuring it works against a real-world docs app like Ember Animated, Mirage, or something else with a moderate amount of complexity.

dfreeman commented 4 years ago

The prember compatibility issue in #327 has been looming large for me recently as we gear up for a big Octane-alignment docs overhaul in ember-cli-typescript.

I'll aim to revisit that this week to see if we can at least solve the blocking part so that prember can be used with AddonDocs. Beyond that, I don't have a super strong opinion about whether prember should be included out of the box, but I'm definitely not opposed.

RobbieTheWagner commented 4 years ago

@dfreeman let me know when you make some progress on that issue, and I can then try to help out with prember integration.

lolmaus commented 4 years ago

Well, this is where it went: https://github.com/empress/field-guide#readme

RobbieTheWagner commented 4 years ago

@dfreeman did you ever get a chance to look into this?

dfreeman commented 4 years ago

@rwwagner90 Apologies! I started in on an implementation back in December and then got completely sidetracked. I need to take a bit of time to look at what I wrote and see if I can remember what I was doing, and I'll report back.

RobbieTheWagner commented 4 years ago

@dfreeman I'm happy to help as well, just let me know! It seems like there are talks of deprecating addon docs, since it is bad for SEO, but I love using it, so I would like to get this working, so it's a viable solution again.

RobbieTheWagner commented 4 years ago

@dfreeman would you like to possibly pair on this? I just took a look for awhile and it seems that the main thing we need to do is move the replacing of the rootUrl to much earlier. I think we could build the app, then if we also want to update latest, we can copy the built output over to the latest folder as part of the ember build, rather than in deploy. I think this would be a good first step at least.