Skip to content

Export: Exclude node_modules and .git paths from site export #1264

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.

Sign up for GitHub

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

Jump to bottom
Merged
ivan-ottinger merged 3 commits into from
Apr 28, 2025
Merged

Export: Exclude node_modules and .git paths from site export #1264

ivan-ottinger merged 3 commits into from
Apr 28, 2025

Conversation

@ivan-ottinger
Copy link
Contributor

@ivan-ottinger ivan-ottinger commented Apr 24, 2025 •
edited
Loading

Related issues

Proposed Changes

  • exclude node_modules and .git paths from site export

Testing Instructions

  1. Check out the PR branch and build the app with npm start.
  2. Open one of your local sites in Finder / Terminal and create a bunch of directories with .git and node_modules with some random files in them.
  3. Add these directories to /wp-content/plugins, /wp-content/themes, etc. ...
  4. Head over to the Import / Export tab and export the site.
  5. Extract the export and review the directories. There should be no .git or node_modules directories present.

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?

@ivan-ottinger ivan-ottinger self-assigned this Apr 24, 2025
ivan-ottinger
ivan-ottinger commented Apr 25, 2025
View reviewed changes
src/lib/import-export/export/exporters/default-exporter.ts
Comment on lines +233 to +234
const isNodeModulesDirectory = relativePath.includes( 'node_modules' );
const isGitDirectory = relativePath.includes( '.git' );
Copy link
Contributor Author

@ivan-ottinger ivan-ottinger Apr 25, 2025

Choose a reason for hiding this comment

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

I think it should be safe to exclude all those paths that include node_modules or .git in them, but happy to make it more strict if you think it's necessary. 🙂

Copy link
Contributor

@gcsecsey gcsecsey Apr 25, 2025

Choose a reason for hiding this comment

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

I agree IMO this should be strict enough! 👌

@ivan-ottinger ivan-ottinger marked this pull request as ready for review April 25, 2025 12:35
@ivan-ottinger ivan-ottinger requested a review from a team April 25, 2025 12:35
epeicher
epeicher approved these changes Apr 25, 2025
View reviewed changes
Copy link
Contributor

@epeicher epeicher left a comment

Choose a reason for hiding this comment

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

I have tested it, and it works as expected. There are no node_modules or .git folders on the exported site. LGTM! :shipit:

Studio folder Exported site folder
CleanShot 2025-04-25 at 16 14 18@2x CleanShot 2025-04-25 at 16 15 56@2x

gcsecsey
gcsecsey approved these changes Apr 25, 2025
View reviewed changes
Copy link
Contributor

@gcsecsey gcsecsey left a comment

Choose a reason for hiding this comment

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

Thanks @ivan-ottinger for working on this and for adding such detailed testing steps! 🙌

The changes LGTM and it's working great for me too! Thanks @epeicher for the handy tree listing command, it made screenshotting much easier! 😄

My site files exported files
CleanShot 2025-04-25 at 16 31 38@2x CleanShot 2025-04-25 at 16 34 19@2x

src/lib/import-export/export/exporters/default-exporter.ts
Comment on lines +233 to +234
const isNodeModulesDirectory = relativePath.includes( 'node_modules' );
const isGitDirectory = relativePath.includes( '.git' );
Copy link
Contributor

@gcsecsey gcsecsey Apr 25, 2025

Choose a reason for hiding this comment

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

I agree IMO this should be strict enough! 👌

@ivan-ottinger
Copy link
Contributor Author

ivan-ottinger commented Apr 28, 2025

Thank you for your review, Roberto and Gergely! 🙂

@ivan-ottinger ivan-ottinger merged commit fa54e1d into trunk Apr 28, 2025
10 checks passed
@ivan-ottinger ivan-ottinger deleted the update/site-export-paths-exclusion branch April 28, 2025 08:25
gavande1 pushed a commit that referenced this pull request Apr 28, 2025
@ivan-ottinger @gavande1
Export: Exclude node_modules and .git paths from site export (#1264)
047d645 
* Exclude `node_modules` from plugin directories during site export

* Remove leftover code

* Exclude node_modules and .git directories from file export in a simpler way
sejas
sejas reviewed Apr 28, 2025
View reviewed changes
Copy link
Member

@sejas sejas left a comment

Choose a reason for hiding this comment

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

Great work! I confirm that the .git and node_modules folders, along with all the files inside them, are excluded. This PR changes the behaviour of Sync and Import/Export features.

I left a comment on STU-411 to update docs in Import/Export mentioning that these folders are excluded.

Some logs I've added

----> Excluding wp-content/plugins/node_modules
----> Excluding wp-content/plugins/node_modules/.DS_Store
----> Excluding wp-content/plugins/node_modules/.git
----> Excluding wp-content/plugins/node_modules/yay
----> Excluding wp-content/plugins/node_modules/yay/test.diff
----> Excluding wp-content/plugins/node_modules/.git/test.diff
----> Excluding wp-content/plugins/mine/.git
----> Excluding wp-content/plugins/mine/.git/test.diff
----> Excluding wp-content/mu-plugins/sqlite-database-integration
----> Excluding wp-content/mu-plugins/sqlite-database-integration/LICENSE
----> Excluding wp-content/mu-plugins/sqlite-database-integration/activate.php
----> Excluding wp-content/mu-plugins/sqlite-database-integration/admin-notices.php
----> Excluding wp-content/mu-plugins/sqlite-database-integration/admin-page.php
----> Excluding wp-content/mu-plugins/sqlite-database-integration/constants.php
----> Excluding wp-content/mu-plugins/sqlite-database-integration/db.copy
----> Excluding wp-content/mu-plugins/sqlite-database-integration/deactivate.php
----> Excluding wp-content/mu-plugins/sqlite-database-integration/health-check.php
----> Excluding wp-content/mu-plugins/sqlite-database-integration/load.php
----> Excluding wp-content/mu-plugins/sqlite-database-integration/php-polyfills.php
----> Excluding wp-content/mu-plugins/sqlite-database-integration/readme.txt
----> Excluding wp-content/mu-plugins/sqlite-database-integration/wp-includes
----> Excluding wp-content/mu-plugins/sqlite-database-integration/wp-includes/sqlite
----> Excluding wp-content/mu-plugins/sqlite-database-integration/wp-includes/sqlite/class-wp-sqlite-db.php
----> Excluding wp-content/mu-plugins/sqlite-database-integration/wp-includes/sqlite/class-wp-sqlite-lexer.php
----> Excluding wp-content/mu-plugins/sqlite-database-integration/wp-includes/sqlite/class-wp-sqlite-pdo-user-defined-functions.php
----> Excluding wp-content/mu-plugins/sqlite-database-integration/wp-includes/sqlite/class-wp-sqlite-query-rewriter.php
----> Excluding wp-content/mu-plugins/sqlite-database-integration/wp-includes/sqlite/class-wp-sqlite-token.php
----> Excluding wp-content/mu-plugins/sqlite-database-integration/wp-includes/sqlite/class-wp-sqlite-translator.php
----> Excluding wp-content/mu-plugins/sqlite-database-integration/wp-includes/sqlite/db.php
----> Excluding wp-content/mu-plugins/sqlite-database-integration/wp-includes/sqlite/install-functions.php

@ivan-ottinger
Copy link
Contributor Author

ivan-ottinger commented Apr 28, 2025

Thanks, Antonio!

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

Reviewers

@sejas sejas sejas left review comments

@epeicher epeicher epeicher approved these changes

@gcsecsey gcsecsey gcsecsey approved these changes

Assignees

@ivan-ottinger ivan-ottinger

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ivan-ottinger @sejas @epeicher @gcsecsey