Skip to content

Assets: Build JS & CSS files #1674

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

Open
wants to merge 17 commits into
base: trunk
Choose a base branch
from
Open

Assets: Build JS & CSS files #1674

wants to merge 17 commits into from

Conversation

obenland
Copy link
Member

@obenland obenland commented May 7, 2025

Building on #1673, this PR adds the JS and CSS files in /assets to the entry points of npm run build.
It minimizes the files and adds RTL support for stylesheets. This PR also adds RTL support to the post preview.

Proposed changes:

  • Adds webpack extention to account for /assets entrypoints.
  • Updates script and style references to use the build versions.
  • Adds RTL support to Post Preview.

After

Screenshot 2025-05-07 at 4 05 25 PM

Changelog entry

  • Automatically create a changelog entry from the details below.
Changelog Entry Details

Significance

  • Patch
  • Minor
  • Major

Type

  • Added - for new features
  • Changed - for changes in existing functionality
  • Deprecated - for soon-to-be removed features
  • Removed - for now removed features
  • Fixed - for any bug fixes
  • Security - in case of vulnerabilities

Message

Post Preview now supports RTL languages.
Embed and wp-admin styles are optimized for RTL languages.

@obenland obenland requested a review from pfefferle May 7, 2025 21:09
@obenland obenland self-assigned this May 7, 2025
pfefferle
pfefferle previously approved these changes May 8, 2025
@obenland obenland force-pushed the add/css-processing branch from 5cbacb0 to 957ab46 Compare May 8, 2025 19:21
@obenland obenland changed the base branch from add/js-lint-format to trunk May 8, 2025 19:21
@obenland obenland dismissed pfefferle’s stale review May 8, 2025 19:21

The base branch was changed.

@obenland obenland requested a review from pfefferle May 8, 2025 19:22
obenland added a commit that referenced this pull request May 8, 2025
@pfefferle
Copy link
Member

pfefferle commented May 10, 2025

This PR seem to change the way how we work with "static" js. Can you maybe add a switch, to use the asset file in dev mode ( maybe using https://developer.wordpress.org/reference/functions/wp_get_environment_type/) and/or write a doc about how to best use it in the daily development process.

@obenland
Copy link
Member Author

Good call on updating the docs!

It does change the way we work with these files, bringing it inline with how blocks are developed. It shouldn't be necessary to switch modes as long as these files get edited while running npm run dev.

With that in mind, I do wonder if it would make sense to move these js and css files into /src, to keep all the files that get built in one place?

@pfefferle
Copy link
Member

let's move them to source to be consistent. maybe we should use a proper folder structure and mimic the block file pattern a bit?

@github-actions github-actions bot added the Docs label May 13, 2025
@obenland
Copy link
Member Author

Yeah, happy to move them to src. I'm not sure there's much benefit to creating a fake block structure around them.

@github-actions github-actions bot added [Block] Federated reply Respond to posts, notes, videos, and other content on the fediverse. [Focus] Editor Changes to the ActivityPub experience in the block editor labels May 13, 2025
@pfefferle
Copy link
Member

pfefferle commented May 13, 2025

Not a fake block structure, but more descriptive folder structures and simpler file names. There is no need to prefix them for example.

@obenland
Copy link
Member Author

obenland commented May 13, 2025

Cool, yeah I think I did that.

@pfefferle
Copy link
Member

pfefferle commented May 14, 2025

I am not sure about the folder structure... This is a bit inconsistent... Shouldn't we reuse the folder structure of the blocks even if it is not a block?

Like:

admin
    ├── header-image.js
    ├── wp-admin.js
    └── style.scss

and

embed
    └── style.scss

@obenland
Copy link
Member Author

Oh, I see! I think we could do something like that. Let me experiment with it a bit.

The limiting factor will be the discoverability of entry points. Currently we can say "any file in /js and /css is an entry point" and don't have to manually add new ones to Webpack. If we were to break it out in arbitrary folders, that would no longer work.

However, we could try moving all blocks into a /blocks folder and say "any file not in /blocks is an entry point" and see if that works.

@pfefferle
Copy link
Member

I would do the /blocks folder "hack"! We have so many points in the code where we decided to be explicit over implicit, so I see no issue to properly configure every new asset we add!?!

obenland and others added 3 commits May 14, 2025 16:38
Also add RTL support for stylesheets and post preview.
@obenland obenland force-pushed the add/css-processing branch from 2ac4725 to 3c33d72 Compare May 14, 2025 21:39
@obenland
Copy link
Member Author

The good news: I managed to make it work to combine scripts and styles in folders and still be recognized as entry points.
The bad news: I forgot to update .prettierignore with the new paths for blocks before building and pushing, so they're now all prettified. I think that should be fine though.

@pfefferle
Copy link
Member

why do the blocks have to be in the blocks subfolder? why not have everything on one level?

@obenland
Copy link
Member Author

why do the blocks have to be in the blocks subfolder? why not have everything on one level?

Practically because blocks are built differently than the other JS/CSS file. It also helps with organization, alongside the wp-admin and embed folders

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Federated reply Respond to posts, notes, videos, and other content on the fediverse. [Block] Follow Me [Block] Followers [Block] Post settings [Block] Reactions [Block] Remote Reply aka "Reply on the Fediverse", in the comment list Docs [Feature] Reactions [Feature] WP Admin [Focus] Editor Changes to the ActivityPub experience in the block editor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants