SharePoint / sp-dev-docs

SharePoint & Viva Connections Developer Documentation
https://docs.microsoft.com/en-us/sharepoint/dev/
Creative Commons Attribution 4.0 International
1.25k stars 1.01k forks source link

The size of the node_modules folder in a project is really big. #59

Closed patmill closed 7 years ago

patmill commented 8 years ago

On my machines, they are appearing as around 233 megs, but I've seen reports of others up to 400 megs or more. Note that the bundles that are downloaded to the browser are fairly small, but the build footprint is quite large.

The first thing is to attempt to rationalize the versions of some packages. There are ~63 megs of projects included at the root as well as in sub node_modules folders. For example, gulp-typescript includes typescript, as well as it being at the top level. The top asks for 1.8.9, and gulp-typescript is requesting 1.8.7

vvolodin commented 8 years ago

And some modules used (on multiple levels) are really outdated.

wictorwilen commented 8 years ago

@vvolodin - that's also the nature of node.js, in order to get that fixed Microsoft and/or we (the community) has to tell those module owners to update their modules. Contact them, create PR's etc.

mauricionr commented 8 years ago

when i run npm dedupe my machine take a lot of time to resolve the duplicated dependencies

vvolodin commented 8 years ago

@wictorwilen Yes, exactly my point, that's why I'm contacting this project owners asking to update their modules :) Project made with this generator references React 0.14.8 and office-ui-fabric-react 0.36.0

SPDoctor commented 8 years ago

If you are seeing folder sizes of 500MB + and deeply nested folders, please check your version of npm - should be something like 3.10.7. If npm -v reports 2.something you need to upgrade: npm -g install npm@next

mike-morr commented 8 years ago

Can we please remove the bug field? This may be an expectation that needs to be set more appropriately, but I think changing the project to accommodate for this is a huge mistake. The sp-pnp-js project is 120mb and it is a relatively small library. This is just the reality of this ecosystem. I would mark it "By Design". I think the workbench should be considered a required dependency. There is a lot of value in that 220mb. @patmill @wictorwilen @SPDoctor

vvolodin commented 8 years ago

Definitely an ecosystem thing. You are not meant to ever copy, move or upload these files.

patmill commented 8 years ago

There are bugs in here as well that need to be fixed. typescript and phantom are showing up multiple times, so that's 30+ megs of stuff that should get cleaned up. I don't think we'll get to 10 megs or anything silly like that, but we can certainly do better than we currently are.

wictorwilen commented 8 years ago

I did a little research here, I have about 220-250 Mb of node_modules, depending on which framework I choose in the generator setup.

But I noticed this:

A non scientific test when removing the stuff above I get down to 165 Mb - it might be hard to get below that, unless you start to rely on global modules.

vvolodin commented 8 years ago

Pretty please, don't remove sourcemaps.

wictorwilen commented 8 years ago

@vvolodin yea, it's a give and take - but do you really need sourcemaps for SPFx? Given that all this is being open sourced, you can pull it down and build a "debug" version if you really need to.

vvolodin commented 8 years ago

They're very useful at the moment - I can debug everything right in the browser (I had to dig around property pane stuff today). If it requires an additional step in the future it's perfectly fine though.

wictorwilen commented 8 years ago

@vvolodin - I'm not saying sourcemaps should be removed for your modules/web parts, rather the built-in stuff

vvolodin commented 8 years ago

Of course, you wouldn't be able to remove my sourcemaps even if you really wanted to :) I was getting familiar with what's under the hood for a couple of days now, I know how it's rendered with React and Flux, how stuff behind property pane is just a mock up using sessionStorage and etc. Sourcemaps of the built-in stuff definitely help with this.

SPDoctor commented 8 years ago

@wictorwilen good point about testing f/w in hello world sample. I'm a TDD advocate but I think it's up to the individual dev (or team) to decide whether they want unit tests, and even if they do they might not make the same test framework choices. I think the SharePoint Framework might benefit from being a little less opinionated. Yeoman generator is a great opportunity to allow developer to make choices about what to include.

patmill commented 8 years ago

@wictorwilen is on the money here (although you missed the fact that we're also including TypeScript twice, so there's another 12-14 megs of savings). We've spent the better part of today discussing various topics around this area. I'll post a new top level issue with our thoughts, and point the various bugs at it.

In a nutshell though, the current idea is 1 - clean up our dependencies to not include multiple versions of the same things 2 - remove some test stuff by default 3 - document what and why we are where we are 4 - new work to make it easier to remove/replace/add build tasks 5 - create a sample and documentation of a hello world webpart that has no build system at all - just the final manifest.json, js file and app package.

mike-morr commented 8 years ago

@patmill Responses numbered the same below.

  1. This is not a bug or an issue. Cleaning up dependencies is cool, but not necessary. npm 3 automatically dedupes. Places with a minor version difference should be cleaned up. The phantomjs definitely should be cleaned up, it's not a bug, but it is big and unnecessary. This is strictly hygiene IMO.
  2. Please reconsider and don't remove anything by default. This thing is perfect for a ton of scenarios just the way it is. It is out of the box ready to go. I prefer a different testing framework, but this thing is dead simple and for the most part just works. I am going to use it until I can build my own boilerplate. I will mostly use this for customers though. I believe this should be on by default, but eventually optional or let the community handle it. You have near exactly what is needed by folks new to the technology right now. The highest priority issue i see is getting the framework dependencies out of the developer's way so they can truly use whatever they want.
  3. This would be awesome.
  4. I am definitely against this. As a PFE in Services you have built something that I can put in front of my customers and it just works with testing out of the box. They might get a preference for a testing framework as they start to transition further into learning TypeScript, but I want them to get started on the right foot. You have integrated the most common a.k.a. documented testing framework. I would have to spend an entire day doing this unless it was an option in the generator. You are already delivering something necessary for .NET folks who are new to this with what you have right now. If we start adding hooks a.k.a. complexity, now they start veering off into the scary world where you spend more time debugging your build stack than writing code for your own project. We need this soft landing. This is near perfect for existing .NET devs who have no preferences yet. :100:
  5. I would prefer to see a sample webpart that has a bare minimum gulp build system (all in one gulpfile.js) that does nothing but compile TS to JS, SASS to CSS with Fabric UI wired up and the minimum code required to compile the manifest and app package. The community can take that and go crazy with it. The TS and SASS would be optional. I can already see the final output in the dist and temp/deploy folders. The community needs to know how to get it into that state.

Just my opinion, but I hope the team will give it careful consideration.

wictorwilen commented 8 years ago

@mike-morrison: given the extremely negative feedback on the number of files and file size, something has to go. Personally, the file count doesn't matter that much to me. But given the feedback, rants and angry smiles on facetwitter this has to be done. And if the generator had an option of "Include testing framework" I think there is a point where the engineering and community can meet. That would allow you to have a smaller footprint for the "hello world" samples.

HughAJWood commented 8 years ago

To achieve a simpler setup process I propose two things

1) Clear guidance for setting up dependencies globally (Some guidance can be found here http://stackoverflow.com/questions/6480549/install-dependencies-globally-and-locally-using-package-json)

2) Creating a Docker container for the whole dev shebang. Downside for Windows this would only be available for newer versions of Windows 10 however (Quote: The current version of Docker for Windows runs on 64bit Windows 10 Pro, Enterprise and Education (1511 November update, Build 10586 or later). In the future we will support more versions of Windows 10.)

3) An alternative to Docker is merely providing a automated installer script however I would recommend using Docker for what it is designed for here.

wictorwilen commented 8 years ago

@HughAJWood - avoid global dependencies if possible, this tends to just cause issues with other stuff. And Docker containers for this is something that the community could/will create imo.

wictorwilen commented 8 years ago

I think this image shows where to start looking as well:

image
HughAJWood commented 8 years ago

@wictorwilen I think the onus should be on Microsoft for creating a contained setup, or at least a simpler setup for this. However a container like Docker removes the issues with global packages. The only real reason is if you are sharing the project. However these should be the base set of dependencies anyway.

wictorwilen commented 8 years ago

@HughAJWood - I'm no Docker expert but don't we just pivot the discussion to the "why is my SPFx Docker container so large" instead then?

Like this: http://stackoverflow.com/questions/24394243/why-are-docker-container-images-so-large :-)

And given the negative feedback on node.js tooling, introducing Docker as well will probably not make that discussion better...

HughAJWood commented 8 years ago

@wictorwilen the docker image would be far smaller than 200MB, with less install time exact size I'm unsure, but gzipping the folders would give an indication it's a similar process. You would still have 200MB on the disk though, but the benefit is either you can delete the container and remake it for a new project which would take seconds, or install global dependencies in the container. Then you mitigate the issue of dependency sharing issue which is caused by global deps. This would then allow the container to be used for multiple projects without issue while allowing easier team sharing cross platform. However the Windows constraint is quite high, this is my only caveat on the solution.

SPDoctor commented 8 years ago

You can use various strategies to deal with gigantic dev setup, sure. But a far better solution is to not have the gigantic install in the first place. I think we solved the 500+ MB problem (npm version). Now we need to keep going identifying dupes and things that can be made optional, and it sounds as though @patmill is onto this :-). Yes, webpack will mean the output is not an issue for the end user, but this thing has to be developer-friendly as well. Some have said "Visual Studio installs huge amounts also", but remember that's why we started using VS Code in the first place. This is a real problem, but totally fixable, and will result in a much better developer experience and more credible story for the broader dev audience and well worth getting right before Ignite.

HughAJWood commented 8 years ago

Raw we have 370MB on disk in our test environment. GZip 93MB, so the docker container would be no more than 120MB. Download was very quick, and unpacking took 1min 20s on my machine.

@SPDoctor if the image was still 120MB but the hello world folder was only a few KB would this be more acceptable? 120MB is pretty much nothing compared to the several GB of Visual Studio. This is the idea I have anyway. Looking at the folder there are no direct duplications either. There are files with same names, and similar sizes, but for example one is for amd processors and other is the standard library and contain very minor code alterations. There are many other duplicate file names, such as array.js but these are array and it's extensions for each library for example lodash. I think 120MB as global dependencies would be the smallest we could get to for now. Remember all the unit tests and licenses etc are also included. I'm currently writing an analysis to check all dependencies so I will post when I have more information.

SPDoctor commented 8 years ago

@HughAJWood Let's not add docker to the list of dependencies. Let's fix this and then docker is a great option for your dev env if you want to use it, but not a necessity. And as I said, the size of VS is why we are here in the first place. If your build is clunky you can hide it by wrapping it with another layer of technology, but much better to fix the build. @patmill's plan is a good one.

HughAJWood commented 8 years ago

@SPDoctor as @mike-morrison has pointed out many of the things cannot be removed further your fix of switching to npm 3 has already done the optimisation here. We still need the environment, yes we can have a smaller hello-world, but really also only if we have a simpler environment and Docker is the solution for that. It's not another technology dependency, it's what Docker was designed to do. We have this problem of a large dependency overhead on the spfx system, but they are mostly required for global operations not for the app. You can't install globally easily without messing up on some machines and some devs would get confused and the process isn't always straight forward. Supplying a Docker container would resolve these globalisation issues and give you your #5 on @patmill's list. The crux is you shouldn't do globalisation of npm dependencies without a container and they are required. Either the community supplies it, which would mean keeping it up to date, or the onus is on Microsoft to supply a stable reproducible development environment in the smallest and simplest footprint possible.

SPDoctor commented 8 years ago

Not suggesting -g for packages. The word "globalization" means something else entirely, and is another thing that could be made optional ;-) (or more correctly localization).

HughAJWood commented 8 years ago

@SPDoctor wasn't thinking about localisation when I said globalisation, I mean to say they are globalised, they are no longer local to the package. But then again you knew that so why am I typing :P

mike-morr commented 8 years ago

@SPDoctor @wictorwilen Why don't we just rename it from HelloWorldWebPart to Spfx-Starter-Kit so it sets the appropriate expectations?

SPDoctor commented 8 years ago

@mike-morrison I think that would help reduce the WTF factor. Developing this idea further, maybe the problem is we are re-installing the tooling for every web part. Could we support building multiple web parts in sub-folders - I know we can already add another web part (albeit with issues still to resolve). So yo sharepoint-framework-starter-kit once, and then yo spfx-starter-webpart each time you want a new one. Just an idea.

chakkaradeep commented 8 years ago

@SPDoctor Yes, you can build multiple web parts in the same project. That is indeed the goal. There are few issues with that in the preview today but we already have a fix internally which should be available in the next drop. The idea is that what you scaffold is a client-side solution which can contain one or more client-side components - the component you have today are web parts.

patmill commented 8 years ago

@SPDoctor - Waldek created a blog post with the workaround here - https://blog.mastykarz.nl/add-second-web-part-sharepoint-framework-project

mike-morr commented 8 years ago

maybe we should add npm dedupe to postinstall for npm 2 users? Perhaps detect the npm version and only dedupe if they are running npm 2. That would also allow a message being displayed recommending they upgrade.

SPDoctor commented 8 years ago

@chakkaradeep, @patmill, yes, aware of that. I was thinking more of multiple projects or multiple client-side solutions where the web parts aren't necessarily connected or deployed together. So by analogy we install Visual Studio once and have multiple projects and solutions. To some extent it feels like we are doing the equivalent of installing Visual Studio for each project/solution. So I was thinking the yo framework scaffolds your dev environment and then a level below that you have your project directories that containing roughly what is currently in the directory src, but one for each project, containing one or more webparts. Deployment would be project by project. Just thinkin' out loud - no analysis to back it up, sorry.

SPDoctor commented 8 years ago

@mike-morrison can't we just make >=npm@3.10.7 a dependency? Presumably when npm 3 becomes npm@latest the problem will go away anyway. Or maybe npm can't depend on itself?

Not sure what we can do for people who already used npm2 as the directory structure has changed with npm 3. Probably easier to start again with a fresh directory.

mike-morr commented 8 years ago

@SPDoctor I don't think you can since it is a global dependency. I could be wrong though. For the people that are already running npm 2 they should be able to run npm dedupe and get a similar reduction in space used.

chakkaradeep commented 8 years ago

@SPDoctor it is certainly not installing VS for every web part. In fact, for every ASP.NET project you create in VS, you get the ASP.NET Nuget packages and other dependencies installed for that web application. Unfortunately, since VS is an offline installation, they include a version (read: not the latest) of those NuGet packages with VS along with a local Nuget store to install/manage those (which applies to many other things in VS as well like Office Developer Tools).

The approaches you see here, of course accepting that there are de-dupes and other optimizations we can do, is a typical client-side web application approach. There are few things here:

All these is how the client-side world works today with many optimizations done in the framework to deploy, load and manage such components in Office 365/SharePoint.

Agree, this is new to many folks and we need to do a better job in documenting this. That is already in motion :)

SPDoctor commented 8 years ago

@chakkaradeep Yes, other client-side frameworks do a similar thing but not as big (not the ones I have used). So I see merit in presenting yo @microsoft/sharepoint as setting up the environment and then build multiple web parts and modules from there, so your last bullet sounds good. At the moment I think folks are getting the message that they need to set up the whole project structure once for each single web part and that is a problem. So cleaning up dupes and possibly a few more things optional is still going to help, and improving the documentation (which is already very good, BTW) - well it sounds like you are dealing with this. Excellent :-D

SPDoctor commented 8 years ago

@mike-morrison Agreed - you can't install npm locally. It can also be an issue if npm3 is not compatible with some other project somewhere on your machine. Dedupe if npm2 sounds like a good idea, but still paying the price of waiting while all the dupes download in the first place - think npm3 is a better option on balance.

Anyway, problem will go away with the passage of time ;-)

HughAJWood commented 8 years ago

@SPDoctor oh compatibility problems you say #docker another problem :P

nickpape commented 8 years ago

We did some work to continue to dedupe the node_modules folder, including most of the work @wictorwilen mentioned:

The new node_modules folder (for a clean npm install) runs approximately 165mb.

It is larger in Mac due to some extra filesystem packages (which come from gulp-core-build-karma).

node_modules

nickpape commented 8 years ago

Not closing this issue, as this will be something we continue to improve.

patmill commented 7 years ago

Closing old issues. We will continue to minimize our module size.

msft-github-bot commented 4 years ago

Issues that have been closed & had no follow-up activity for at least 7 days are automatically locked. Please refer to our wiki for more details, including how to remediate this action if you feel this was done prematurely or in error: Issue List: Our approach to locked issues