ember-fastboot / fastboot-express-middleware Goto Github PK

I'm not sure what the status of HTTP 2.0 Server Push support in express.js is actually but if it supports it shouldn't we be able to take advantage of it by pushing the assets to the client along with the prerendered HTML?

Maybe there's an obvious reason why we don't do that yet that I'm just not seeing - if so, please go ahead and close this and sorry for the noise.

The devDependency express was updated from 4.16.4 to 4.17.0.

🚨 View failing branch.

This version is covered by your current version range and after updating it in your project the build failed.

express is a devDependency of this project. It might not break your production code or affect downstream projects, but probably breaks your build or test tools, which may prevent deploying or publishing.

Your Greenkeeper Bot 🌴

I was working on a feature that used an npm package to get the IP address of a client conncting to the Fastboot server, which was configured as a middleware in fastboot-server.js, which initializes Fastboot using this middleware. However, it appears that there is no way to pass data to Fastboot through fastboot-express-middleware, which makes it very challenging to extend Fastboot using other middlewares.

Let me know if you need more information about my use case, but overall I think it would be important to maintain modularity by allowing information to be passed into Fastboot through this middleware.

Hi 👋,

the fastboot 2+ dependency is already included it the package.json 🎉.

Would it make sense to release a new version, so ember-cli-fastboot and fastboot-app-server can be updated too?

Not used anywhere. 🤷‍♂️

https://github.com/ember-fastboot/fastboot-express-middleware/blob/master/src/index.js#L26

Calling all knowledgeable Express.js folks!

The test case in this repo that tests resilient mode without a custom error handler should fail (eg, it does IRL when using resilient mode without a custom error handler), but for some reason it passes.

My impression from reading the docs is that the default Express error handler will render the built-in error page when it gets called, which will in turn subvert the resilient behavior.

So you actually must define an error handler like

app.use(function (err, req, res, next) {
  console.error(err.stack)
  next();
})

To essentially ignore the error and allow resilient mode to continue.

But the tests don't reflect that. Are they somehow turning off the default error handler in express?

Hi 👋,

would it make sense to release a patch version with #60?

Thanks

/cc @rwjblue

It's been a while. 😄

Currently when we set value resilient: true, fastboot-express-middleware show undefined on distPath with code:

app.get('/*', fastbootMiddleware('/path/to/dist', {
  resilient: true
}));

Code above show options paramter on constructor in path file: node_modules/fastboot/src/index.js:

{distPath: undefined, resilient: true}

I have update it with code:

app.get('/*', fastbootMiddleware({
  distPath: '/path/to/dist',
  resilient: true
}));

Code above show options paramter on constructor in path file: node_modules/fastboot/src/index.js:

{distPath:  '/path/to/dist', resilient: true}

It work fine to me.

Please help check it again.

Thanks,

FastBoot provides us with an API to append headers to the response. If you set the same header more than once, FastBoot will store the values in an array. However, the Express middleware then loops through the flattened array of header entries and sets them using res.set(), which overwrites any duplicate headers with the last value.

Example

If you wanted to return several cookies, you might try the following:

let responseHeaders = this.get('fastboot.response.headers');
responseHeaders.append('set-cookie', 'first_cookie=...; path=/');
responseHeaders.append('set-cookie', 'second_cookie=...; path=/');

The first cookie gets overwritten by the second in the response.

In the wild

Both ember-simple-auth and ember-cookies seem to expect the cookies to be appended, not overwritten with the last value.

In ember-cookies: https://github.com/simplabs/ember-cookies/blob/0a0fb0e4d85b70041258829c5e7e2e4f1359ca7b/addon/services/cookies.js#L94-L105

  _writeFastBootCookie(name, value, options = {}) {
    let responseHeaders = this.get('_fastBoot.response.headers');
    let serializedCookie = this._serializeCookie(...arguments);


    if (!isEmpty(options.maxAge)) {
      options.maxAge *= 1000;
    }


    this._cacheFastBootCookie(...arguments);


    responseHeaders.append('set-cookie', serializedCookie);
  },

This innocuous code then produces subtle bugs in code like this in ember-simple-auth: https://github.com/simplabs/ember-simple-auth/blob/master/addon/session-stores/cookie.js#L246

this.get('_cookies').write(this.get('cookieName'), value, cookieOptions);
if (!isEmpty(expiration)) {
  let expirationCookieName = `${this.get('cookieName')}-expiration_time`;
  let cachedExpirationTime = this.get('_cookies').read(expirationCookieName);
  this.get('_cookies').write(expirationCookieName, this.get('cookieExpirationTime') || cachedExpirationTime, cookieOptions);
}

ember-simple-auth depends on the cookie store in FastBoot, so if you set an expiration date on your auth cookies, this expiration cookie will overwrite your auth cookie.

I assume this is not really the intended behaviour, especially for the append method of the fetch header API. I know Express previously provided all sorts of patches, including automatically appending duplicate header values (https://github.com/expressjs/express/wiki/Migrating-from-3.x-to-4.x#ressetheaderset-cookie-val).

The best solution I can come up with would be to handle the cookies with res.cookie here and let users pass in cookies via the FastBoot service. That's gonna require changes in 3 repos at least, so not sure how people feel about that.

We can filter out the cookie headers and set them all at once (which correctly sets multiple set-cookie headers), but we don't want to send more than one cookie with the same name. ember-simple-auth in particular will write the auth cookie 3-4 times for some reason. So, we'd have to parse the serialized cookies to check for duplicates – doesn't sound like a great idea.

The dependency fastboot was updated from 2.0.2 to 2.0.3.

🚨 View failing branch.

This version is covered by your current version range and after updating it in your project the build failed.

fastboot is a direct dependency of this project, and it is very likely causing it to break. If other packages depend on yours, this update is probably also breaking those in turn.

Your Greenkeeper Bot 🌴

After upgrading to Ember 2.7 (which included deprecation of baseURL) and starting an Ember app using Fastboot, the app tries to retrieve assets based on the rootURL specified, e.g.

• rootURL/assets/vendor.css
• rootURL/assets/appName.js

However, Fastboot is still serving these assets at

• assets/vendor.css
• assets/appName.js

My use case is that I want to cache the HTML that is generated from Fastboot. I use fastboot-express-middleware as middleware with fastboot-app-server. This has an afterFastboot hook, which is only accessible on an error because index.js line 51 res.send(html) just returns the HTML rather than calling next().

What I currently do is replace that line with next() and pass through the html on the response object.

Is this something that might be good to change, and how would it affect users not using fastboot-app-server?

Happy to make a pull request if you did think this was a valid change.

The devDependency request-promise was updated from 4.2.2 to 4.2.3.

🚨 View failing branch.

This version is covered by your current version range and after updating it in your project the build failed.

request-promise is a devDependency of this project. It might not break your production code or affect downstream projects, but probably breaks your build or test tools, which may prevent deploying or publishing.

Your Greenkeeper Bot 🌴