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

Update Isolated Block Editor #94

Merged
merged 4 commits into from
Dec 2, 2021
Merged

Update Isolated Block Editor #94

merged 4 commits into from
Dec 2, 2021

Conversation

ice9js
Copy link
Member

@ice9js ice9js commented Nov 25, 2021

This patch updates the isolated block editor to the latest version as well as updating all @wordpress/* package numbers to match.

Unfortunately, I ran into some trouble with Automattic/isolated-block-editor@2.7.0, which seems to have a dependency on @wordpress/block-editor@7.0.4 which is out of sync with its own dependencies that rely on version 8.0.5 of the same package.
Because we're bundling @wordpress/* packages together with our apps, this was causing our editor to break whenever trying to load any core blocks.

As a quick fix, I made a new branch on the isolated block editor repo and bumped the version manually, then pointed our dependency at that commit. This allows us to move forward without waiting for the next release.

This patch will enable #64 to finally be merged.

Testing

Apply the patch, run yarn install and verify our apps build correctly, especially @crowdsignal/dashboard and @crowdsignal/project-renderer.

@ice9js
Copy link
Member Author

ice9js commented Nov 26, 2021

After talking with John, he opened Automattic/isolated-block-editor#91 to adjust the package versions.

I think we should best wait for that to get merged and just update to the next version or update to that branch instead of mine. Let's keep this PR on hold in the mean time.

@ice9js ice9js mentioned this pull request Nov 26, 2021
@mirka
Copy link
Member

mirka commented Nov 29, 2021

Unfortunately, I ran into some trouble with Automattic/isolated-block-editor@2.7.0, which seems to have a dependency on @wordpress/block-editor@7.0.4 which is out of sync with its own dependencies that rely on version 8.0.5 of the same package.

👋 I've been trying to add more integration tests to iso-editor to prevent issues like this, could you help me understand what happened here? For example if you have a simple repro case that triggers this issue, we can add that to the tests. (cc @johngodley)

@ice9js
Copy link
Member Author

ice9js commented Nov 29, 2021

Hi @mirka! Thank you for taking a look :)

In our case, the issue here was specific to the version 2.7.0 of the isolated-block-editor which was causing two different versions of @wordpress/block-editor to be installed. As we rely on the bundled packages rather than providing Gutenberg externally, this caused all core blocks to crash whenever added to the editor.

If you'd like to try to reproduce this in our environment and investigate, you can do so by checking out bd2b48fcbb08bc7557c2198bec10065b4a5a7414 and following these steps:

  • Run yarn install
  • Add 127.0.0.1 crowdsignal.localhost to your hosts file
  • Run yarn workspace @crowdsignal/dashboard start and wait for webpack to start
  • Go to https://crowdsignal.localhost/project to open the editor
  • Attempting to add any core blocks will cause them to crash due to missing/conflicting modules

I hope this helps :)

@mirka
Copy link
Member

mirka commented Nov 29, 2021

Ok, so the issue was that iso-editor's @wordpress/block-editor version conflicted with crowdsignal-ui's @wordpress/block-editor version? I think I misread "out of sync with its own dependencies" to mean that iso-editor's dependencies were somehow internally inconsistent. (Natural languages are ambiguous!)

In that case I guess the only thing we can do on iso-editor's side is to be quicker with wp dep updates. Time to whip out the ol' Renovate 🤖

@ice9js
Copy link
Member Author

ice9js commented Nov 30, 2021

I think I misread "out of sync with its own dependencies" to mean that iso-editor's dependencies were somehow internally inconsistent. (Natural languages are ambiguous!)

No, that part was correct. In 2.7.0, the isolated block editor specifies @wordpress/block-editor @ 7.0.4. But other dependencies, @wordpress/edit-post to give an example, depend on @wordpress/block-editor @ 8.0.5.
This is causing two versions of that and some other wordpress packages to be installed causing inconsistencies and eventually failure.

When I bumped @wordpress/block-editor to 8.0.5 (here) the issue was resolved and only a single version of each package was installed.

Now that I think of it, that might be a good point to test - that there's only a single version of each package when you install it.

In that case I guess the only thing we can do on iso-editor's side is to be quicker with wp dep updates. Time to whip out the ol' Renovate 🤖

Yeah, for the time being we do rely on the Gutenberg version that comes with isolated-block-editor so keeping it up to date is definitely really appreciated on our end.
That said, we do realize everybody has got their priorities so now that I'm thinking... maybe if we could formalize the process a little (including testing), it'd be easier for us to prepare everything and help out when we need an update like that.

@mirka
Copy link
Member

mirka commented Nov 30, 2021

No, that part was correct. In 2.7.0, the isolated block editor specifies @wordpress/block-editor @ 7.0.4. But other dependencies, @wordpress/edit-post to give an example, depend on @wordpress/block-editor @ 8.0.5. This is causing two versions of that and some other wordpress packages to be installed causing inconsistencies and eventually failure.

Ah, I kind of see what happened here. Based on yarn.locks, iso-editor is internally consistent (edit-post 5.0.4 → block-editor 7.0.4), but because of version ranges (^) crowdsignal actually installs edit-post 5.0.11block-editor 8.0.5 😬

https://github.com/Automattic/isolated-block-editor/blob/5791a6ae0f616df4b109c42e209ec67f8f17e684/yarn.lock#L4286-L4294

crowdsignal-ui/yarn.lock

Lines 5356 to 5364 in 645ff81

"@wordpress/edit-post@^5.0.4":
version "5.0.11"
resolved "https://registry.yarnpkg.com/@wordpress/edit-post/-/edit-post-5.0.11.tgz#0c0bb0805c5f20cc3b659c1632c610f81aeb5edb"
integrity sha512-UVFve7ETYdJjDm2TgAJKOxiRmoTYNu75fkbvTluxnKbvWKDXzY0QCpw47JY4g+JmYBRda6ZGV51eY5P5G5BZ6Q==
dependencies:
"@babel/runtime" "^7.16.0"
"@wordpress/a11y" "^3.2.4"
"@wordpress/api-fetch" "^5.2.6"
"@wordpress/block-editor" "^8.0.5"

This seems like a pretty gnarly problem that is bound to happen again, but I don't know what the best course of action is. Like, is this something that can or should be prevented on the Gutenberg side? Or should iso-editor be hard pinning its wp dep versions? 🤷

Copy link
Contributor

@CGastrell CGastrell left a comment

Choose a reason for hiding this comment

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

LGTM! Both apps work.

Should we fix the dependencies' versions? Maybe not right now, but ...

@ice9js ice9js merged commit 5616f54 into main Dec 2, 2021
@ice9js ice9js deleted the update/iso branch December 2, 2021 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants