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

Block.json files aren't rebuilt when running npm run dev #16104

Closed
talldan opened this issue Jun 12, 2019 · 4 comments · Fixed by #16150
Closed

Block.json files aren't rebuilt when running npm run dev #16104

talldan opened this issue Jun 12, 2019 · 4 comments · Fixed by #16150
Assignees
Labels
[Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended [Type] Build Tooling Issues or PRs related to build tooling

Comments

@talldan
Copy link
Contributor

talldan commented Jun 12, 2019

Describe the bug
When running npm run dev and making changes to a block.json files, the files are not rebuilt.

The watch script does notice the change as the isSourceFile function returns true for a json file:
https://github.com/WordPress/gutenberg/blob/master/bin/packages/watch.js

However, the build worker script doesn't handle json files in its BUILD_TASK_BY_EXTENSION object and ignores the change:
https://github.com/WordPress/gutenberg/blob/master/bin/packages/build-worker.js#L85

To reproduce
Steps to reproduce the behavior:

  1. Run npm run dev
  2. Update a block.json file
  3. Observe that those changes had no effect

Expected behavior
Changes to block.json files should be rebuilt by npm run dev

@talldan talldan changed the title Block.json ( Block.json files aren't rebuilt when running npm run dev Jun 12, 2019
@talldan talldan added [Type] Bug An existing feature does not function as intended [Type] Build Tooling Issues or PRs related to build tooling labels Jun 12, 2019
@swissspidy
Copy link
Member

Previously: #15135 / #15455.

@talldan
Copy link
Contributor Author

talldan commented Jun 12, 2019

Looked into it some more. Usually when babel builds a js file, it outputs the compiled version into [package]/build-module/[path-to-file].

For json files, it's inlining them (I think using https://www.npmjs.com/package/babel-plugin-inline-json-import).

The json for a block.json is inlined into the index.js it's imported by. This can be seen if you look at `block-library/build-module/[block]/index.js, the metadata from the json file is all there.

The challenge for watching is that when a change to the block.json file is detected, it's not that file that needs to be rebuilt, but the neighbouring index.js. To handle this gracefully for any json files in the project you'd have to do a sort of reverse trace of where the file is imported, and rebuild those files.

For block.json files they're such an established pattern that it might be worth adding a special rule to the build scripts for them. It wouldn't expand to other json files, but would at least solve the most pressing use-case.

Another solution would be to transform the json into js, so that there's an equivalent block.js file in the build-modules folder, and then all json files could be handled that way. Is that what you're proposing @gziolo for #16088?

@gziolo
Copy link
Member

gziolo commented Jun 12, 2019

For block.json files they're such an established pattern that it might be worth adding a special rule to the build scripts for them. It wouldn't expand to other json files, but would at least solve the most pressing use-case.

It's all based on convention, so we could assume that the change in block.json requires to update the corresponding index.js file in the same folder. It isn't perfect but would resolve this issue.

Another solution would be to transform the json into js, so that there's an equivalent block.js file in the build-modules folder, and then all json files could be handled that way. Is that what you're proposing @gziolo for #16088?

It's a different proposal, it's still based on reading JSON file and inlining it into the JS file which imports it. With macros you can teach webpack to never cache files transpiled with Babel which use them: facebook/create-react-app@11737bc. However, I don't know how it works in practice. It's mostly the issue on our side because we use a custom watch solution to build packages.

@talldan talldan self-assigned this Jun 13, 2019
@talldan talldan added the [Status] In Progress Tracking issues with work in progress label Jun 13, 2019
@talldan
Copy link
Contributor Author

talldan commented Jun 13, 2019

I'll try to tackle this now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended [Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants