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

Fix #16659 - Unwanted node modules copying #17331

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

nul800sebastiaan
Copy link
Member

Prerequisites

  • I have added steps to test this contribution in the description below

If there's an existing issue for this PR then this fixes #16659

Description

As noted in #16659, everything from App_Plugins is copied into the web output. This is unneeded and in the case of the node_modules directory unwanted behavior, these files are sources and not output that Umbracco needs to serve packages/extensions.

  • The old behavior has led to many of the "path too long" errors we've been seeing while building extensions.
  • It also makes the regular dotnet build step needlessly long on the first run as it needs to copy over loads of small files.
  • This also affects dotnet publish commands, the whole node_modules folder is included in the published output that people will try to put on a server, again leading to very slow copying times AND path too long errors

The new behavior avoids all these problems.

Reproduction

  • Set up a v15 RC2 site
  • Add these files in the directory where the csproj file is App_Plugins.zip
  • Run a dotnet build for this csproj file and note that in bin\Debug\net9.0\App_Plugins\vanilla-extension a node_modules folder appears
  • Completely delete the bin\Debug folder
  • Check out this PR locally and build the NuGet packages with the command: dotnet pack --configuration Release --output .\release\
    • Note: run this command in the directory where umbraco.sln is located, you will see a release directory appearing which will contain the NuGet outputs for Umbraco
  • Copy the path of the release directory produced above, for example: E:\Dev\Umbraco-CMS15\release
  • In your existing v15 RC2 site, where the csproj file is, run the following command: dotnet add package Umbraco.Cms --prerelease --source E:\Dev\Umbraco-CMS15\release (the --source path is the one you copied in the last step).
  • Now run another dotnet build and note that the node_modules directory is NOT in the bin\Debug\net9.0\App_Plugins\vanilla-extension directory

Here's a video of the steps I took to verify the before and after behavior:

fix16659.mp4

As noted in #16659, everything from `App_Plugins` is copied into the web
output. This is unneeded and in the case of the `node_modules` directory
unwanted behavior, these files are sources and not output that Umbracco
needs to serve packages/extensions.

The old behavior has led to many of the "path too long" errors we've
been seeing while building extensions.
@nul800sebastiaan
Copy link
Member Author

This should be cherry-picked to v14 when merged as well.

Note: I ran into this because I was following the new Extending the backoffice training. I had problems building because of the infamous "path too long" error. I already have long path names enabled on my machine, so that does not always fix the problem.

@iOvergaard iOvergaard merged commit 2ff0a3f into v15/dev Oct 29, 2024
14 of 18 checks passed
@iOvergaard iOvergaard deleted the fix/issue16659/unwanted-node-modules-copying branch October 29, 2024 22:06
nikolajlauridsen pushed a commit that referenced this pull request Nov 4, 2024
As noted in #16659, everything from `App_Plugins` is copied into the web
output. This is unneeded and in the case of the `node_modules` directory
unwanted behavior, these files are sources and not output that Umbracco
needs to serve packages/extensions.

The old behavior has led to many of the "path too long" errors we've
been seeing while building extensions.
@nikolajlauridsen
Copy link
Contributor

Cherry-picked into the release 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants