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

FastBoot still renders the body on redirect #802

Open
SergeAstapov opened this issue Oct 28, 2020 · 4 comments
Open

FastBoot still renders the body on redirect #802

SergeAstapov opened this issue Oct 28, 2020 · 4 comments

Comments

@SergeAstapov
Copy link
Contributor

SergeAstapov commented Oct 28, 2020

Looks like it's a miss in the implementation as outlined in #195 (comment):

transitionTo still renders the body. The middleware or fastboot could be modified to not include it, but a better solution would be to tell Ember to abort the page rendering. Tips on how to do that would be appreciated. I couldn't find anything straightforward in the docs.

fastboot library does not include body for redirects, implemented in commit ember-fastboot/fastboot@3f386fd.

Since then lot of things have changed in Ember rendering engine and I hope this is feasible/possible to implement to tell Ember not to render anything.

Looks like shouldRender configuration option would result in desired outcome but it's set too early in bootstrap phase.

Any other ideas?

@kiwiupover
Copy link
Member

Hello @SergeAstapov can you provide a recreation of this issue.

I would be happy to jump on a call to understand your issues.

@SergeAstapov
Copy link
Contributor Author

SergeAstapov commented Dec 7, 2020

Hi @kiwiupover! Sorry for delay, link to simple reproduction is https://github.com/SergeAstapov/fastboot-repro-802

Actual steps to minimal reproduction are simple, here is the commit which adds two routes where A route does redirect to B route.

I added console.log() statement to model hook for routes A and B and by navigating to route A via curl you can see route B actually get invoked (console.log() from model hook runs) + template content gets rendered.

Screen Shot 2020-12-06 at 11 14 49 PM

The rendered html from Ember gets ignored and simple redirect message returned by Result here.

Ideally, I would expect Ember not to follow redirects in FastBoot mode and do not render any DOM in FastBoot mode for redirects as it's simply ignored and not used for the response.

@SergeAstapov
Copy link
Contributor Author

SergeAstapov commented Apr 20, 2021

Hi @kiwiupover! Given the above explanation and reproduction, do you have enough information on the issue? Happy to provide more info if needed.

For the future readers should they end up in the same situation:

Currently we end up setting a flag on ApplicationController from the redirect hook of the respective route route like so

export default class RouteA extends Route {
  ...
  redirect (model) {
    if (/* some redirect condition based on model */ && this.fastboot.isFastBoot) {
      getOwner(this).lookup('controller:application').shouldRender = false;
      this.transitionTo('route-b');
    }
  }
}

given ApplicationController has default value for shouldRender:

# controllers/application.js
export default class ApplicationController extends Controller {
  shouldRender = true;
}

and then in application.hbs we wrap all the content into condition

{{#if this.shouldRender}}
  ...
{{/if}}

@hoIIer
Copy link
Contributor

hoIIer commented Aug 13, 2021

@SergeAstapov @kiwiupover just hit this same error when adding a redirect for a base route... and it exposes my prod s3 bucket when fastboot returns the redirect body from above!

Ideally I'd like to not have to wrap my application in a flag, is there a settled way to handle this in 2021?

router.js

  ...
  this.route('legal', function() {
    this.route('tos');
  });
import Route from '@ember/routing/route';

export default class LegalIndexRoute extends Route {
  redirect() {
    this.transitionTo('legal.tos');
  }
}

going to site.com/legal will render the redirect body instead of the actual ember redirected page.

edit: after looking at the source code, found a way around it.

this._body = `<h1>Redirecting to <a href="${location}">${location}</a></h1>`;

for redirects visit().then((response) => response.html()) hits L40

so essentially you have to do something like this

          // render the page with fastboot.
          const res = await this.fastboot.visit(
            this.request.url,
            {request: this.request},
          );

          if (res.statusCode >= 300 && res.statusCode <= 399) {
            html = await res.domContents().body;
          } else {
            html = await html.html();
          }

not sure how common it is to render fastboot page manually like this but perhaps an optional arg to html() to render redirects could be helpful

Additionally this is in the context of rendering a fastboot app inside a cloud cdn and not through a node server.

edit 2

the above didn't work as I naively didn't realize I was only getting the html body from domContents().body (duh)

so far the only thing I can think of is a monkey patch/hack

          const res = await this.fastboot.visit(
            this.request.url,
            {request: this.request},
          );

          if (res.statusCode >= 300 && res.statusCode <= 399) {
            // trick fastboot into thinking it was a successful request.
            res._fastbootInfo.response.statusCode = 200;
          }

          // render the pages html.
          html = await res.html();

could open a pr

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

No branches or pull requests

3 participants