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

Change copying PHP files to dist directory to opt-in via a CLI flag. #39171

Merged
merged 6 commits into from Mar 3, 2022

Conversation

ryanwelcher
Copy link
Contributor

Description

Based on feedback post #38715 being merged, this PR changes the approach around copying PHP files by moving to an opt-in approach via providing a --webpack-copy-php flag to either the start or build commands. By moving to opt-in, we should be able to avoid more unintentional issues with existing projects while still allowing the functionality to be accessed.

Testing Instructions

I created a block using npx @wordpress/create-block@ my-first-dynamic-block --template=@ryanwelcher/dynamic-block-template ( this contains an template.php file in the src directory )
Then ran node {FULLPATH}/gutenberg/packages/scripts/bin/wp-scripts.js -- build --webpack-copy-php

Types of changes

Changes the approach for copying PHP files to be opt-in via a CLI flag.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).
  • I've updated related schemas if appropriate.

@ryanwelcher ryanwelcher changed the title Try/add copy flag to scripts Change copying PHP files to dist directory to opt-in via a CLI flag. Mar 2, 2022
@ryanwelcher ryanwelcher added [Package] Scripts /packages/scripts [Type] Enhancement A suggestion for improvement. Developer Experience Ideas about improving block and theme developer experience labels Mar 2, 2022
@gziolo gziolo added this to Needs review in Core JS Mar 2, 2022
Copy link

@pauarge pauarge left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

Thank you @ryanwelcher for providing this fix. Let's keep it optional for now and maybe we can figure out some ways where we could extend the block.json metadata in a way that allows automatically detecting necessary PHP block files.

@gziolo gziolo merged commit b1f2064 into WordPress:trunk Mar 3, 2022
@github-actions github-actions bot added this to the Gutenberg 12.8 milestone Mar 3, 2022
@gziolo gziolo moved this from Needs review to Done in Core JS Mar 3, 2022
gziolo added a commit that referenced this pull request Mar 3, 2022
…39171)

* Enable accepting --webpack-copy-php flag from the CLI commands

* Update the glob pattern based on existence of --webpack-copy-php flag.

* Update README and changelog files.

* Add PR to the changelog.

* Update CHANGELOG.md

* Update README.md

Co-authored-by: Greg Ziółkowski <grzegorz@gziolo.pl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Developer Experience Ideas about improving block and theme developer experience [Package] Scripts /packages/scripts [Type] Enhancement A suggestion for improvement.
Projects
No open projects
Core JS
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants