aurelia / ssr-engine

The server-side rendering engine for Aurelia.
MIT License
16 stars 6 forks source link

Document preboot options for clarity #8

Open rquast opened 6 years ago

rquast commented 6 years ago

Minification is deprecated in version 6 of angular preboot. This leaves ssr-engine open to breaking changes in the future if it is not removed from all server examples. Not only this, but any change that angular introduce to preboot could introduce breaking changes. In a separate issue, I've recommend forking preboot and maintaining an aurelia version (https://github.com/aurelia/ssr-engine/issues/9). Also, either document or add an interface for the preboot options because it's a bit of a black box when the options are set in server.js in your project, then sent to the middleware, then sent to ssr-engine, then sent to the preboot transformer, then sent to preboot. Currently interfaces.ts in ssr-engine has no type information:

  /**
   * Options that are passed to preboot
   */
  prebootOptions?: any;

Desired action: Remove breaking changes in the ssr skeletons, and add sufficient comments in the code as to how options are passed to preboot (which I should really be doing in my ssr fork). This should be in ssr-engine, middleware-koa and the ssr skeleton.

Alexander-Taran commented 6 years ago

@rquast this is not actionable. Needs background context, reasoning and desired behavior. Otherwise it'll sit here till the end of days

fkleuver commented 6 years ago

He brings up an interesting point though. From the angular preboot docs:

  • minify (deprecated) - minification has been removed in v6. Minification should be handled by the end-user

So this is something that from the side of aurelia-srr-engine will need to be either agreed or disagreed with, taking into account the potential maintenance burden and such.

rquast commented 6 years ago

Added PrebootOptions interface to my ssr-engine fork. https://github.com/rquast/ssr-engine/commit/05a9f996d4159fea61426052c2d8d11f3d2f968e

EisenbergEffect commented 6 years ago

What would be the point of minification in this context?

EisenbergEffect commented 6 years ago

@rquast Are you hoping to send a PR with the improvements you are making? It would be great to have another person helping to work on this part of the ssr implementation.

rquast commented 6 years ago

@EisenbergEffect Sure, I've just been reading up on your commit guidelines. Will keep looking into it and ask for feedback before sending any PRs.

EisenbergEffect commented 6 years ago

Sounds awesome!

rquast commented 6 years ago

Sorry I just checked the ssr skeleton for minification and it wasn't there. I must have added it to my config at some point because the preboot wasn't minifying. You're right, it doesn't belong in this context which is why they deprecated it. To help other people in the future, I'm also adding this to the server.js example in my skeleton:

const PrebootOptions = require('preboot').defaultOptions;
app.use(aureliaKoaMiddleware({
  preboot: true,
  prebootOptions: Object.assign({}, PrebootOptions),
  bundlePath: require.resolve(bundle),
  template: require('fs').readFileSync(path.resolve('./dist/index.html'), 'utf-8')
}, {
  main: () => {
    delete require.cache[require.resolve(bundle)];
    return require(bundle);
  }
}));

That way you can quickly click through and find out what the options are and where they are from.