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

Ensure WP Dependencies correctly generated. #132

Merged
merged 1 commit into from Jun 8, 2022
Merged

Ensure WP Dependencies correctly generated. #132

merged 1 commit into from Jun 8, 2022

Conversation

peterwilsoncc
Copy link
Contributor

Description of the Change

This updated the JavaScript file to add WordPress dependencies using the import syntax rather then the exposed functions within the wp namespace.

This change allows wp-scripts to pick detect the dependencies correctly and include them in the generated file build/index.asset.php.

Closes #131

Alternate Designs

Alternative considered: manually adding the dependencies in the PHP file.

I chose this approach as it maintains automation while working around the error we were seeing in the last configuration.

Possible Drawbacks

None that I am aware of.

Verification Process

  1. check out new branch
  2. run npm i; npm run build;
  3. confirm all WP dependencies are listed in the build/index.asset.php file

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Changelog Entry

Fixed: Ensure required WordPress libraries were included as dependencies.

Credits

Props @peterwilsoncc

@peterwilsoncc peterwilsoncc requested review from a team and iamdharmesh and removed request for a team June 8, 2022 01:16
@peterwilsoncc peterwilsoncc self-assigned this Jun 8, 2022
@jeffpaul jeffpaul added this to the 1.1.0 milestone Jun 8, 2022
Copy link
Member

@iamdharmesh iamdharmesh left a comment

Choose a reason for hiding this comment

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

LGTM 🚀. Thanks for fix this @peterwilsoncc

@jeffpaul jeffpaul merged commit 52d1244 into develop Jun 8, 2022
@jeffpaul jeffpaul deleted the fix/131 branch June 8, 2022 11:33
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.

Asset dependecies calculated incorrectly.
4 participants