Skip to content

SSR Guide: Remove app.js #1297

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

Merged
merged 8 commits into from
Oct 26, 2021
Merged

Conversation

CyberAP
Copy link
Contributor

@CyberAP CyberAP commented Oct 26, 2021

Description of Problem

Right now SSR Guide uses an app.js as a common place to create an SSR app. I think this is a more than ok approach for prototyping, but for an actual production code it also has two issues that I'd like to solve:

  1. Each integration (like vue-router for example) with an app could be different from another. These types of integration could differ significantly depending on the environment (server or client). That adds complexity to the app.js code, which should know that it can be used in both environments and have a condition for each case that is different on client\server.
  2. The code for server and client is not tree-shaken from the client build. This could be changed by defining a variable that's always false for the client build so it might actually tree-shake these dependencies. I would consider that a hack rather than a proper solution to the problem.

Proposed Solution

We can disregard app.js and write client and server instantiation code separately from each other. That way we solve two problems at once: no need for special handling for each type of client\server integration, automatic full tree-shaking.

Additional Information

I also think this is a good place to start for everyone trying out SSR for the first time, since if this in fact limits the codebase significantly (for example there's a case of a lot of code duplication) you can easily switch to the current approach.

Copy link
Member

@NataliaTepluhina NataliaTepluhina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CyberAP great work! I've left a bunch of nitpicks regarding trailing commas but rest of the changes look good to me 👍🏻

@CyberAP
Copy link
Contributor Author

CyberAP commented Oct 26, 2021

@NataliaTepluhina thank you! 👐 I also fixed all the rest of the typos and code style that I could find.

@NataliaTepluhina
Copy link
Member

@CyberAP awesome! Let's get it merged

@NataliaTepluhina NataliaTepluhina merged commit 210b792 into vuejs:master Oct 26, 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 this pull request may close these issues.

2 participants