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

Easy "ng build" improvements to make "platform":"server" work well on Windows & Visual Studio #7903

Closed
SteveSandersonMS opened this issue Oct 2, 2017 · 6 comments · Fixed by #7937
Assignees
Labels
feature Issue that requests a new feature

Comments

@SteveSandersonMS
Copy link

SteveSandersonMS commented Oct 2, 2017

I work on the ASP.NET team and am looking at changing the default Visual Studio ASP.NET + Angular templates to be based around Angular CLI. One thing that's stopping us right now is issues with publishing and deploying such Angular CLI apps.

Basically, for ng build to work well with Universal on Windows/VS, it's essential for it to have an option to produce self-contained builds, i.e., ones that don't rely you to deploy node_modules to your production server.

Why this is needed

Originally, the default official ASP.NET templates for Angular, which have server-side rendering on by default, expected developers to deploy node_modules to their production servers. This led to many, many issue reports, because it turns out that from Windows, people cannot deploy node_modules reliably. This isn't really developers fault. Two basic problems:

  • On Windows, path length limitations mean that MSBuild fails to publish deep nested folder structures, such as those that appear in node_modules.
  • Visual Studio's MSDeploy mechanism processes every file one-by-one quite slowly, so trying to deploy 20,000+ files from node_modules is a process that has you waiting for an hour and very frequently will fail with a network-related error eventually.

So in the default ASP.NET Core Angular template, we solved this by changing the default Webpack config not to treat node_modules as a Webpack external, causing Webpack to bundle all the server-side rendering dependencies into its single bundle output file. That output file is then large (4MB+) but that's OK: people can reliably deploy one 4MB file, as opposed to 20,000 tiny files. And it's only used server-side - this file is never sent to browsers.

Now we'd like to change the default Visual Studio ASP.NET templates for Angular to use Angular CLI, but we can only do this when the resulting projects can be published and deployed reliably. And that means a self-contained build option.

Fortunately, the implementation for this in Angular CLI is pretty much trivial - see below.

Bug Report or Feature Request (mark with an x)

- [ ] bug report -> please search issues before submitting
- [x] feature request

Versions.

1.4.2

Repro steps.

If you run ng build to build an app configured with "platform":"server", then observe that the resulting dist directory's bundle files are not self-contained. That is, they only include the application's own code - they don't include the @angular code from NPM or any other required packages. The only way it can work at runtime is if you have node_modules on your production server, which causes great difficulty in practice as described above.

Desired functionality.

Ideally there would be an option for the "platform":"server" builds to be self-contained - i.e., can run without any node_modules, simply by not telling Webpack to treat those files as externals during the build. The use cases is to simplify publishing and deployment, especially on Windows/VS.

Mention any other details that might be useful.

This is pretty simple to implement. In server.ts from line 13, we see the Webpack config for externals used in server builds. All we need is a build option that disables this, i.e., leads to the externals array being left empty. Then Webpack will produce completely self-contained bundles that work correctly without node_modules being on the production server.

One minor complication is that, for some reason, Angular CLI tries to minify (via uglify) the server-side bundle in production, which leads to a build error with self-contained bundles. I recommend that server-side builds should not minify by default, because (1) that will fix the error, and (2) who wants to minify their server-side code anyway? It's much easier to understand/debug unminifed server-side code. If necessary this could be another option, but I really think it would be enough just to avoid minifying server builds altogether.

@hansl
Copy link
Contributor

hansl commented Oct 2, 2017

Hello @SteveSandersonMS,

And that means a self-contained build option.

Thank you for providing an actual use case for this option. When designing the Universal, we found a lot of problems with using a self-contained build (I'll explain in a bit), and couldn't possibly find reasons to provide an option for bundling up. So it was decided to remove the option entirely.

Adding the option itself is trivial (as you mentioned), but there is a large cost on the developer and you're turning your project into a minefield. You're basically painting yourself in a corner, for the following reasons:

  1. every dependency used need to be the same within the bundle and outside of it. That means that if you are using RxJS in your backend and passing a reference to an Observable to your application, that Observable will not be compatible (unless you go the extra length to export RxJS from your bundle).
  2. This also means that if you want to replace providers (which the platform-server provides), you wouldn't be able to do it outside the bundle anymore. A good example is replacing the Router code with something application specific (e.g. to redirect routes).
  3. We do not provide you with server code (the express code itself). There are too many way to build a backend and we don't want to be a generic build tool for your server.
  4. It's relatively trivial to create a webpack or rollup configuration that bundles your server code, once the application itself is built from the CLI.

Given all that, it was significantly dangerous for users for us to not allow self-contained builds. So we decided that there should simply not be an option. Remember that the CLI builds the application bundle, but it's up to the developer to bring their own server. As a result, we do not know how to bundle the backend code (we're not a generic build tool, after all), and I'd like you to consider point 4 above and add a rollup configuration to separate building and bundling the application into 2 separate steps.

That being said, if you know what your server is going to be, or if your server can be exclusively in main.server.ts (ie. just render as a string and returns it), then self-contained builds are possible. They're still not a great idea, but they can work.

Therefore, I'm personally okay with adding a flag that will build a fully bundled server application, with showing up a warning that the user needs to know what they're doing. This is something though that will need to be discussed internally with our relationship team, as it has a lot of impacts on usability of universal.

For frameworks like ASP.Net that do not run within NodeJS as part of the server, this might be good enough and help you move along.

@SteveSandersonMS
Copy link
Author

SteveSandersonMS commented Oct 3, 2017

@hansl - thanks for your feedback!

Yes, I totally understand that for Node-based application servers, it's preferable for bundles not to be self-contained, since developers have no choice but to deploy node_modules anyway, and might frequently try to reference their bundle from other Node modules. All the drawbacks you mention make sense in that case. I'm glad you also recognize the need to allow for self-contained bundles in other scenarios (e.g., main application server is not Node based, but instead just calls into the bundled code for prerendering), since without this, experience shows that Windows+VS devs will commonly be unable to deploy their apps to production successfully in practice.

What's the best way to proceed with this? Do you want me to send a PR? I'm very happy to do so if that would help. Or given that the key thing is more about designing how this should appear and be described in help text, is it something you'd prefer to do yourself?

I'd like you to consider point 4 above and add a rollup configuration to separate building and bundling the application into 2 separate steps.

I understand that's technically possible, but we already have templates with a working Webpack config, and people get troubled about how complex it seems to them. The central value proposition around moving to an Angular CLI-based template is to reduce the number of pieces. If we have to introduce a further layer of Webpack/Rollup on top of this, that doesn't really achieve the goal. Our target market includes developers who are totally new to all these technologies, so every moving part is a real cost.

@Brocco Brocco added the feature Issue that requests a new feature label Oct 3, 2017
@hansl
Copy link
Contributor

hansl commented Oct 4, 2017

FYI. I talked to the concerned parties and we decided internally that having an additional flag for this isn't a problem. It will only affect server applications, and default to false. Thinking of along the lines of --externals=none, with default --externals=node_modules. That way we can have other versions if need be (like angular_only) Still tinkering with the name, if you have any suggestions (naming is hard).

@SteveSandersonMS
Copy link
Author

Great to hear it! The name externals for this option sounds ideal to me.

If it's possible for this to be configured in .angular-cli.json as well as being a command-line option that would be perfect, but at least having it as a command-line option would be enough.

hansl added a commit to hansl/angular-cli that referenced this issue Oct 5, 2017
This flag allows people who know what theyre doing to bundle the
server build together with all dependencies. It should only be
used when the whole rendering process is part of the main.ts
or one of its dependencies.

Fixes angular#7903.
hansl added a commit to hansl/angular-cli that referenced this issue Oct 5, 2017
This flag allows people who know what theyre doing to bundle the
server build together with all dependencies. It should only be
used when the whole rendering process is part of the main.ts
or one of its dependencies.

Fixes angular#7903.
hansl added a commit to hansl/angular-cli that referenced this issue Oct 5, 2017
This flag allows people who know what theyre doing to bundle the
server build together with all dependencies. It should only be
used when the whole rendering process is part of the main.ts
or one of its dependencies.

Fixes angular#7903.
hansl added a commit that referenced this issue Oct 5, 2017
This flag allows people who know what theyre doing to bundle the
server build together with all dependencies. It should only be
used when the whole rendering process is part of the main.ts
or one of its dependencies.

Fixes #7903.
Brocco pushed a commit that referenced this issue Oct 5, 2017
This flag allows people who know what theyre doing to bundle the
server build together with all dependencies. It should only be
used when the whole rendering process is part of the main.ts
or one of its dependencies.

Fixes #7903.
@naveedahmed1
Copy link

@hansl can you please confirm if this option is still supported? Since 1.5.0 --bundle-dependencies=all doesn't seem to work as reported here #8616
ASP.Net team plans to ship the final version of Angular CLI based template in January, it would be really great if we could have this option working again. As mentioned by @SteveSandersonMS on Windows the path length limitations prevents MSBuild to publish deep nested folder structures, such as those that appear in node_modules directory. Currently this seems to be the only hurdle that could prevent .Net developers from using this template for large production apps that want to take advantage of SSR.

Thank you for the awesome work :)

dond2clouds pushed a commit to d2clouds/speedray-cli that referenced this issue Apr 23, 2018
This flag allows people who know what theyre doing to bundle the
server build together with all dependencies. It should only be
used when the whole rendering process is part of the main.ts
or one of its dependencies.

Fixes angular#7903.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature Issue that requests a new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants