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

Circular dependency between blockly and dev-tools #5834

Open
alschmiedt opened this issue Dec 20, 2021 · 1 comment
Open

Circular dependency between blockly and dev-tools #5834

alschmiedt opened this issue Dec 20, 2021 · 1 comment
Assignees
Labels
internal External contributions not accepted issue: feature request Describes a new feature and why it should be added

Comments

@alschmiedt
Copy link
Contributor

alschmiedt commented Dec 20, 2021

Is your feature request related to a problem? Please describe.
Having the advanced playground in core creates a circular dependency between Blockly and dev-tools. In the past release we had changes in dev-tools that required the new version of Blockly and Blockly needed the new version of dev-tools for the advanced playground to work. This required us to release a patch version of Blockly to up date the dev-tools version. On top of that, during the quarter we must release a beta version of dev-tools every time that we up date it to rely on the most recent release.

Describe the solution you'd like
I'm not entirely sure on the correct fix for this. The obvious option is to remove the advanced-playground from core and have developers use the advanced playground in samples (probably in the dev-tools plugin?). However, linking to Blockly locally requires the developer builds and packages Blockly every time there is a change. We would probably want to find a way to automate this so that using the advanced playground with local Blockly does not become a large pain point.

Describe alternatives you've considered

Additional context
I believe most if not all of our codelabs tell developers to use the advanced playground. So this will also need to be addressed.

@alschmiedt alschmiedt added issue: feature request Describes a new feature and why it should be added internal External contributions not accepted labels Dec 20, 2021
@maribethb
Copy link
Contributor

maribethb commented Mar 3, 2022

I wrote a brainstorming/options doc and then we discussed as a group. I'll summarize the 3 options discussed.

  1. Don't do anything. This is painful for us when we have a release that involves breaking changes to something in dev-tools, but it's ultimately not super harmful for developers. The extra patch release is annoying to have to do, but it only affects people developing in core blockly, not people using blockly as a dependency (since dev-tools is a dev-dependency, it's only installed when working directly in core). If we stop doing breaking changes or if releases were easier/automated then this option would be even less painful.
  2. Move the advanced playground and other relevant pieces like debug renderer and test blocks into core. We would export these under a separate module in the normal blockly npm package (similar to how the non-JS generators are packaged - you'd have to import the playground specifically when desired, it wouldn't be included in the default import). This would allow us to easily use the advanced playground both within plugins in blockly-samples (since all plugins already depend on core) and from core itself. Nobody was excited about this because although it would not increase the size of the Blockly module served to users by projects that are using modules, it would make the Blockly npm package bigger in general and would increase bytes served to users from projects using Blockly through a CDN. Also, it runs counter to our goal of moving non-essential pieces out of core. We'd have to keep in mind backwards compatibility issues, but those are solvable.
    2a. The same option, but publish the playground as a separate package on npm. This would effectively make core a monorepo with two packages which we strongly do not want to manage, so this is a no go.
  3. Do not use the advanced playground in core. If you really need to use them together, you could start the dev-tools plugin after using npm link to link the local version of core Blockly to the dev-tools plugin. But the default for testing core would be to just use the simple playground. A lot of the team only uses the simple playground and doesn't mind this. Some of the team regularly uses the advanced playground and this option makes them sad. Also, we think there is a risk of adding more features to the simple playground to account for the missing advanced playground, which would be redundant work. Or that tests that would become more difficult to run (e.g. test blocks tests) may not be run as often which would lead to QA issues. Plus, as Abby noted above, many of the codelabs rely on modifying code inside the advanced playground, so those would need to be updated.

Nobody was really strongly in favor of either option 2 or 3. Therefore we decided to go with option 1, but leaving it open to changing our minds in the future. If this continues to be a problem or cause undue amounts of toil, then we will revisit. When we revisit, we are leaning towards Option 3. We can make the npm link process less painful by adding a workflow that automatically repackages blockly after a change, or possibly by enabling blockly to be linked at the top-level instead of from within the dist folder, so that you wouldn't need to run npm run package after each change in core.

Separately, we also should go ahead and change the codelabs so that none of them encourage people to make edits anywhere in the core codebase (unless it is a codelab about contributing to Blockly). Either the codelab should provide starter code, or the codelab should start with calling the create-package script which would generate starter code. This work should be done regardless of what we decide here, but it would also make it easier to go with option 3 in the future.

TLDR: For at least Q1 2022, we do not plan to change the location of the advanced playground. We may decide to do so in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal External contributions not accepted issue: feature request Describes a new feature and why it should be added
Projects
None yet
Development

No branches or pull requests

2 participants