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

Mobile App: Avoid code references that create cyclic dependencies #27751

Open
1 of 4 tasks
gziolo opened this issue Dec 16, 2020 · 1 comment
Open
1 of 4 tasks

Mobile App: Avoid code references that create cyclic dependencies #27751

gziolo opened this issue Dec 16, 2020 · 1 comment
Labels
Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) Needs Dev Ready for, and needs developer efforts [Type] Code Quality Issues or PRs that relate to code quality

Comments

@gziolo
Copy link
Member

gziolo commented Dec 16, 2020

Originally reported by @rafaelgalani when working on #27414 - refactoring to replace the store name wth the exposed store definition for @wordpress/edit-post package. The related part of the comment:

The problem now looks like a cyclic dependency problem... Though I'm not sure of it. That is what I could understand by looking at the stack in the action's log:

FAIL packages/block-library/src/list/test/edit.native.js
  ● Test suite failed to run

    TypeError: Cannot read property 'SETTINGS_DEFAULTS' of undefined



 ---> at Object.SETTINGS_DEFAULTS (packages/block-editor/src/index.js:24:22)
      at Object.<anonymous> (packages/editor/src/store/defaults.js:25:5)
      at Object.<anonymous> (packages/editor/src/store/reducer.js:15:1)
      at Object.<anonymous> (packages/editor/src/store/reducer.native.js:14:1)
      at Object.<anonymous> (packages/editor/src/store/index.js:10:1)
      at Object.<anonymous> (packages/editor/src/index.native.js:13:1)
      at Object.<anonymous> (packages/edit-post/src/editor.native.js:12:1)
      at Object.<anonymous> (packages/edit-post/src/index.native.js:13:1)
      at Object.<anonymous> (packages/components/src/mobile/link-settings/index.native.js:10:1)
      at Object.<anonymous> (packages/components/src/index.native.js:78:1)
      at Object.<anonymous> (packages/rich-text/src/component/index.native.js:20:1)
      at Object.<anonymous> (packages/rich-text/src/index.js:37:1)
 ---> at Object.<anonymous> (packages/block-editor/src/index.js:4:1)   
      at Object.<anonymous> (packages/block-library/src/list/edit.js:6:1)
      at Object.<anonymous> (packages/block-library/src/list/test/edit.native.js:9:1)

There are some architectural issues in React Native files that this PR uncovers. The packages should depend on each other in the following direction:

  • edit-post
    • block-library
      • editor
        • blocks
        • components
          • element

There is also some mix in the picture as edit-post itself might be using one of the deeper dependencies directly, and so on.

We left hardcoded strings outside of the @wordpress/edit-post package and plan to fix all of the issues separately. There are other high-level packages: edit-site, edit-widgets, or edit-navigation that shouldn't be referenced in other parts of Gutenberg core as well.

Since @wordpress/edit-post is an entry point, it should never be the dependency of other core components (it's a different story for 3rd party plugins that customize UI). I'm aware it's a bit of gray area for stores as they are usually consumed inside components so basically it works. There is this issue though that codebase is tightly coupled to the specific screen - edit post. As soon as you will want to support other screens for things like a site, theme templates, or maybe post comments, it will become a blocker. We don't have a solid solution for that, so far we were using props or editor settings for passing down information where necessary. It feels like we need some sort of scope detection to make it more robust. If you have some ideas, I'm happy to hear your thoughts.

Packages to fix

You can search for core/edit-post string in the following packages to discover those issues:

  • @wordpress/components
  • @wordpress/block-directory
  • @wordpress/block-library
  • @wordpress/editor

All those packages should find a different way to obtain data from @wordpress/edit-post. /cc @Tug @hypest

@gziolo gziolo added Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) Needs Dev Ready for, and needs developer efforts [Type] Code Quality Issues or PRs that relate to code quality labels Dec 16, 2020
@geriux
Copy link
Member

geriux commented May 17, 2024

@wordpress/components

Has been addressed in #61102

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) Needs Dev Ready for, and needs developer efforts [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

No branches or pull requests

2 participants