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

Embroider integration #59

Closed
josemarluedke opened this issue Sep 6, 2020 · 10 comments · Fixed by #64
Closed

Embroider integration #59

josemarluedke opened this issue Sep 6, 2020 · 10 comments · Fixed by #64

Comments

@josemarluedke
Copy link

josemarluedke commented Sep 6, 2020

It seems that Prember doesn't work with Embroider.

Here is a PR to an app adding Embroider. Without Embroider, prember works file.

@ef4
Copy link
Owner

ef4 commented Sep 7, 2020

There are two main blockers to resolve. One is already fixed, but depends on getting ember-cli-fastboot to use a newer fastboot package. That’s enough to get full fastboot support under embroider.

The second issue is kicking off the prember build. It uses postprocessTree all, but that is not supported under embroider.

@simonihmig
Copy link
Contributor

One is already fixed, but depends on getting ember-cli-fastboot to use a newer fastboot package. That’s enough to get full fastboot support under embroider.

There is a 3.0.0-beta.2 release now that contains this.

The second issue is kicking off the prember build. It uses postprocessTree all, but that is not supported under embroider.

@ef4 any plan how to fix this one?

@simonihmig
Copy link
Contributor

Ok, so I looked into this a bit more on my own, and my understanding is as follows:

Prember needs to run after the whole app has been built, as FastBoot obviously needs that to visit the configured URLs. That's why in a classic ember-cli build the postprocessTree hook for type="all" is used.

However that hook is not run in Embroider (at least for "all") as you said, and that for good reasons: for Embroider it would need to be called after all stages have run through (including the final webpack run, which yields the executable app). But it seems odd to have v1 addons run after that again, so after the initial compat stage, basically introducing another compat stage running after webpack. And not only just "odd", but potentially harmful as Ed has mentioned elsewhere IIRC, as addons might do things to final output that can break things (like lazy loading) in an Embroider context. Which is probably not the case for prember, but maybe for other addons. So that's not a way forward it seems.

Another option would be to decouple the prerendering from the actual build pipeline, as we don't really need to mangle any existing files during the build process, rather we just add the prerendered html files into the dist folder. So we could do that as a separate run after the actual ember build, e.g. in a postbuild package.json script. And that's what one of the alternative prerendering solutions ember-cli-prerender does.

That addon however seems to be not really maintained actively, and would require some work to put into, like making sure it works with latest Ember and FastBoot versions. Also prember seems to have emerged as the go-to solution for prerendering Ember apps, so it would be nice to give existing users an easier transition path.

So maybe we could add that postbuild approach to prember as an alternative way to integrate it into Embroider based apps. I think it should be possible to mostly reuse the existing infrastructure (e.g. the prerender broccoli plugin) here, and "just" add a ember-cli command that triggers that separate prerendering build step, which users would have to call manually (or add to postbuild). I think it should also be possible to still support the existing APIs, like that urls(For|From}Prember stuff.

As we currently have a use case for making Embroider and prember work together, I should be able to spend some time on it to make it work. Doesn't sound that difficult really, just hope I didn't miss anything important. But would love some feedback and approval for that approach first. @ef4 ?

@ef4
Copy link
Owner

ef4 commented Nov 2, 2020

You can also wrap the return value of ember-cli-build.js in whatever broccoli transform you want, including the one that would do the prember build. That is probably the simplest implementation.

@simonihmig
Copy link
Contributor

Here is a WIP PR: #64

@mansona
Copy link
Collaborator

mansona commented Nov 11, 2020

hey folks 👋 sorry I haven't had a chance to look at this discussion yet, it's been a bit of a busy few months lately 🙃

So if I'm reading the discussion correctly (and the note that you have put on the PR that you're working on @simonihmig) for prember to continue to work you need to make manual changes to your ember-cli-build.js file to manually run the pre-rendering?

I'm wondering if there are any alternative solutions to this, because this might be a big blow to the Empress ecosystem if you can't just install an addon and it starts building 😞

@simonihmig
Copy link
Contributor

I'm wondering if there are any alternative solutions to this

As it stands today, the answer is no. Or at least nothing which "just works", as the postprocess hook (for type=all) that prember currently uses is not run in Embroider. And that is by choice AFAIK. Ed will certainly be able to give you a much better explanation for this, but my rough understanding is that giving you the power to fiddle with the final build output in arbitrary ways can be considered unsafe in an Embroider world, where the output of webpack should be seen like a "black box".

Having said that, I think what prember does is ok in this context, as it only needs the final build output so it can run the compiled app in FastBoot, but it does not modify any existing files, it only adds the prerendered HTML files in its broccoli transform.

Maybe there will be room for some new integration points that allow prember and similar addons to "just work", without the caveats that we have with postprocess, idk?

In the meantime, it's still good I think to have a less elegant but functioning workaround, for those that are keen enough to leave the main road and try embroider out while it's still in a beta/experimental state (which it is). Which is what the PR tries to provide...

because this might be a big blow to the Empress ecosystem if you can't just install an addon and it starts building

Mind that you need to change your ember-cli-build.js anyway to enable Embroider, so it's "just" another call wrapped around the other embroider calls! But yes, with this workaround, you do have to remember doing that when prember is in use.

@ef4
Copy link
Owner

ef4 commented Nov 15, 2020

I would add that the output isn’t completely a black box — it’s supposed to be valid HTML+JS that will make the app run. And we have some tiny extensions to that to account for also making it something fastboot can run. But any valid HTML and JS are fair game.

It’s not clear how we would make a general API for running after the bundler builds, because there are pretty different ways to define “build”, at least in development mode. For example, snowpack doesn’t really build at all, it’s just a one-to-one module rewriting web server, and I do want those architectures to work.

But prember is not really a huge development-time concern anyway. In dev, you get the fastboot server to interact with, live. Maybe prember should become an ember-cli-deploy plug-in.

@sandstrom
Copy link
Contributor

sandstrom commented Jan 5, 2021

@ef4 I don't think prember make sense as a ember-cli-deploy plug-in. ember-cli-deploy is good for many things but it's built to be a part of the deploy flow.

In our case we're building artifacts, we then have other code (that isn't ember-aware) doing the deploys. For us, using prember as a stand-alone thing makes much more sense (if it was baked into ember-cli-deploy we probably wouldn't use it).

In my view, something along what simon said above would make a lot of sense:

Another option would be to decouple the prerendering from the actual build pipeline, as we don't really need to mangle any existing files during the build process, rather we just add the prerendered html files into the dist folder. So we could do that as a separate run after the actual ember build, e.g. in a postbuild package.json script. […]

So maybe we could add that postbuild approach to prember as an alternative way to integrate it into Embroider based apps. I think it should be possible to mostly reuse the existing infrastructure (e.g. the prerender broccoli plugin) here, and "just" add a ember-cli command that triggers that separate prerendering build step, which users would have to call manually (or add to postbuild).

Awesome work on prember btw! 🏅

@mydea
Copy link

mydea commented Dec 7, 2021

FYI after running into some limitations with fastboot (mainly the requirement to have some modifiers like render modifiers run), I wrote an addon with a different prerendering strategy:
https://github.com/mydea/ember-prerender

It can be used stand-alone (use ember prerender instead of ember build), or integrated with ember-cli-deploy. It is still in an early stage but seems to be working fine for our use cases.

Technically, it uses Puppeteer to prerender the pages, instead of fastboot. Since it runs in a "normal" browser, all the regular ember app functionality works as expected. And since the actual serialization/rehydration stuff is actually in Glimmer and not in Fastboot itself, we can nicely leverage the same functionality (awesome architecture!).

I have not tested it yet, but that should work just fine with embroider (because it does not use any special build functionality, it just builds the app normally, then serves it on a local web server and scrapes the pages with Puppeteer). Might be interesting to some! (It also has some downsides, e.g. it is much harder to test the prerendering functionality locally as you can't just do ember serve). Feedback is also welcome!

@ef4 ef4 closed this as completed in #64 Dec 23, 2021
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 a pull request may close this issue.

6 participants