search

cedaro / satispress

Expose installed WordPress plugins and themes as Composer packages.
521 stars 51 forks source link

Remove `node_modules` from dirs to be excluded from archives #173

Closed tyrann0us closed 2 years ago

tyrann0us commented 2 years ago

I know it's possible to change folders to be excluded from archives using the satispress_archive_excludes filter. However, I would like to question the usefulness of the node_module entry:

https://github.com/cedaro/satispress/blob/fef698408272e24777cadb78e70348493d099cb3/src/Archiver.php#L146

Premium plugins/themes are archived by the developer. It is likely[citation needed] that in this process development files and folders like the node_modules folder are already removed by the developer, either manually or by build tools. So if it's kept, it's probably a deliberate decision by the developer.

SatisPress should not interfere with the existence of the node_modules folder. After all, it would also be present if the package were installed manually and not via SatisPress.

In the present case, there are problems (fatal errors) because the node_modules folder does not exist and the relevant plugin ("Borlabs Cookie") is not smart enough to check its existence.

tyrann0us commented 2 years ago

If you think about it, IMHO SatisPress should not have any influence on the archive content at all. It is responsible for providing Composer repositories for premium packages, not for manipulating their content. What do you think, @bradyvercher?

bradyvercher commented 2 years ago

If you think about it, IMHO SatisPress should not have any influence on the archive content at all. It is responsible for providing composer repositories for premium packages, not for manipulating their content. What do you think, @bradyvercher?

I do agree and that's one of the reasons opting to use the artifact provided by the vendor is preferred over zipping from source. node_modules was excluded because it adds a lot of cruft and isn't usually needed. It can be unexpected during development to add a plugin to the repo and wind up with rather large artifacts in the cache directory.

The only time those excludes should really affect anything is when a package is initially added to the repo -- every other time they should be exactly what the vendor provides.

Excluding node_modules seemed innocuous, but if it's causing issues, then I don't see any harm in removing that from the excludes list.