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

Tooling: npm run dev doesn't watch for changes to the base-styles package #55568

Open
afercia opened this issue Oct 24, 2023 · 4 comments
Open
Labels
Developer Experience Ideas about improving block and theme developer experience [Type] Build Tooling Issues or PRs related to build tooling [Type] Developer Documentation Documentation for developers

Comments

@afercia
Copy link
Contributor

afercia commented Oct 24, 2023

Description

Noticed while working on #55547

Pinging @t-hamano @kevin940726 as I saw you recently contributed to this part of the codebase and certainly you are more familiat than me with it.

While working on #55547 I had the need to change something in the base styls and I was surprised to not see my changes reflected in my browseer. Turned out that when running npm run dev, any change to the base style is not 'watched'.

@aristath @SergeyBiryukov and myself spent ~2 hours in a joined troubleshooting session to understand why.

To our understanding, this is in some way by design. The base-styles package isn't included in the watched files. We are guessing that is because any change there would require to rebuild all the scss files where the bast styles are used? Anyways, the developer expeerience is less than ideal.

Te root causes are:

  • hasModuleField returns true only if a package.json file has a module property. This is not the case for the base-styles package.
  • isSourceFile returns true only if the checked file is within a src directory. The base-styles package doesn't have a src directory.

Although these files are built when running npm run build, the lack of a src directory is what makes us think not watching them when running npm run dev is by design. However, as mentioned earlier, the developer experience is less than ideal. As a contributor, I would expect that npm run dev watches all the files necessary to actually code anc copntribute to this repository.

WordPress.org is an open source project that historically focused on making contributing easy and open to everyone.

At the very least we'd think a couple things should be taken into consideration>

== Documentation
I couldn't find any reference in the documentation that mentions npm run dev will not watch for changes to the base-styles packages. As watching these files is supposed to work, such an exception should be documented in a very prominent way. The documentation just says:

There are two ways to build your code. While developing, you probably will want to use npm run dev to run continuous builds automatically as source files change.

The fact base-styles is not 'watched' is not mentioned anywhere, as far as I can tell:

To make contributing easy for everyone and not just something for initiated contributors, this should be docuemnted very clear to avoid contributors spend their time trying to understand why they don't see their changes reflected in teh browser.

== Consider to watch these files
We'd tend to think these files should be 'watched' anyways. By running npm run dev, contributors expect that everything 'just works'. However, there is at least one notable, undocumented, exception. This seems less than ideal for a smooth contributing experience.

Step-by-step reproduction instructions

  • Run npm run dev in your terminal.
  • Edit any file in the base-styles package and save your changes.
  • Observe your terminal: nothing happens.

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@afercia afercia added [Type] Developer Documentation Documentation for developers [Type] Build Tooling Issues or PRs related to build tooling Developer Experience Ideas about improving block and theme developer experience labels Oct 24, 2023
@kevin940726
Copy link
Member

I don't know much about this package nor the npm run dev scripts nowadays unfortunately. Maybe @gziolo would know?

@carolinan carolinan changed the title Tooling: nom run dev doesn't watch for changes to the base-styles package Tooling: npm run dev doesn't watch for changes to the base-styles package Oct 25, 2023
@t-hamano
Copy link
Contributor

I'm not familiar with Gutenberg's build process, but it seems like the isSourceFile() function expects the *.scss file to be in the src directory 🤔

@afercia
Copy link
Contributor Author

afercia commented Oct 27, 2023

@t-hamano thanks. Yes as I mentioned above:

hasModuleField returns true only if a package.json file has a module property. This is not the case for the base-styles package.
isSourceFile returns true only if the checked file is within a src directory. The base-styles package doesn't have a src directory.

Those are the technical reasons.

However, the point is:

  • Why is this not documented? It forced us to spend almost 2 hours to understand why our changes were not visible. This doesn't help making contributing a pleasant experience and I'd say it's unfair towards all contributors.
  • Why this package is not watched in the first place? If performance is a concern, I don't think it's a big one. I can understand the challenge would be rebuilding all the stylesheets when a file in the base-styles package is edited. I think it is possible to find a way to solve it.

Otherwise, the documentation should clearly state in capital letters:

There are two ways to build your code. While developing, you probably will want to use npm run dev to run continuous builds automatically as source files change. All packages will be watched and built as necessary EXCEPT THE BASE-STYLES PACKAGE. {include instructions on how to develop for the base-styles package here).

@gziolo
Copy link
Member

gziolo commented Oct 31, 2023

I'm not familiar with Gutenberg's build process, but it seems like the isSourceFile() function expects the *.scss file to be in the src directory 🤔

Fixing the function seems like the way to go. @wordpress/base-styles is a special package as it contains SCSS source files that are consumed in other places without any modifications, so there is no need for any build folders. I don't think anyone predicted the implications on the npm run dev that runs in the watch mode. These files don't change often so it's probably a reason it didn't come up earlier as an issue. Fixing the function, and leaving a code comment documenting the exception should be enough.

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 [Type] Build Tooling Issues or PRs related to build tooling [Type] Developer Documentation Documentation for developers
Projects
None yet
Development

No branches or pull requests

4 participants