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

Feature: add @wordpress/scripts build process for GiveWP 3.0 #6862

Merged
merged 6 commits into from
Aug 8, 2023

Conversation

jonwaldstein
Copy link
Contributor

@jonwaldstein jonwaldstein commented Aug 2, 2023

Description

This adds @wordpress/scripts for building our v3 forms related scripts. The entry point for these scripts are in wepback.config.js.

Affects

This only affects new scripts being added to wepback.config.js

Visuals

N/A

Testing Instructions

  • npm run dev:v3
  • npm run build:v3
  • npm run watch:v3
  • npm run build runs both mix and wp-scripts for production

Pre-review Checklist

  • Acceptance criteria satisfied and marked in related issue
  • Relevant @unreleased tags included in DocBlocks
  • Includes unit tests
  • Reviewed by the designer (if follows a design)
  • Self Review of code and UX completed

@jonwaldstein jonwaldstein marked this pull request as ready for review August 7, 2023 17:41
@jonwaldstein jonwaldstein requested review from JasonTheAdams, ravinderk and kjohnson and removed request for JasonTheAdams and ravinderk August 7, 2023 17:46
@jonwaldstein
Copy link
Contributor Author

@JasonTheAdams @kjohnson this is ready for review.

cc: @ravinderk this is how we'll be handling v3 related form scripts including new gateways.

Copy link
Contributor

@JasonTheAdams JasonTheAdams left a comment

Choose a reason for hiding this comment

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

Good work, @jonwaldstein! I had a couple minor questions and one suggestion.

Comment on lines +65 to +68
# @wordpress/scripts build directory
build
**/build.*
**/build.asset.*
Copy link
Contributor

Choose a reason for hiding this comment

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

Should anything be added to the .distignore file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so, I assume we want to keep the build folder for distribution since it contains the generated scripts.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, the /build folder probably needs to be included in the distribution, instead of trying to mess with moving that target directory.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jonwaldstein For sure! I'm really thinking about if there are any specific files (file maps?) within the build that shouldn't be included in distribution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotchya, it appears wp scripts does not generate those file maps so we should be good. If anything looks weird during distribution we can add to the distignore.

@jonwaldstein
Copy link
Contributor Author

@JasonTheAdams ready for re-review

Copy link
Contributor

@JasonTheAdams JasonTheAdams 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, @jonwaldstein! Nice to have this done for the merge!

@jonwaldstein jonwaldstein merged commit 94dc806 into develop Aug 8, 2023
16 checks passed
@jonwaldstein jonwaldstein deleted the feature/wordpress-scripts branch August 8, 2023 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants