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-dynamic-connection plugin blocks are not compatible with the generator functions for the blocks they replace #2442

Open
1 task done
cpcallen opened this issue Aug 8, 2024 · 2 comments
Labels
type: bug Something isn't working
Milestone

Comments

@cpcallen
Copy link
Contributor

cpcallen commented Aug 8, 2024

Check for duplicates

  • I have searched for similar issues before opening a new one.

Component

The block-dynamic-connection plugin

Description

The existence of the overrideOldBlockDefinitions method in the block-dynamic-connections plugin API suggests that the block definitions in this plugin can be used as drop-in replacements for the blocks they replace, but

  • the blocks provided by the plugin are not compatible with the block generator functions for the blocks they replace, and
  • the plugin does not provide (and therefore overrideOldBlockDefinitions cannot install) block generator functions for the dynamic blocks.

Reproduction steps

  1. Go to the plugin's test page
  2. Drag a dynamic_text_join block to the workspace.
  3. Add one, two or three text blocks to the inputs of the join block.
  4. Click on the JavaScript tab of the playground output panel.
  5. Observe that the code generated for the join block is always [].join('');, regardless of attached text blocks.

Additional Info

  • The specific incompatibility in dynamic_text_join vs text_join is that the former has a count property named itemCount while in the latter it is named itemCount_.
  • It is probably that similar incompatibilities exist for the other dynamic blocks.
  • It is probably possible to rewrite the dynamic blocks to be generator-compatible with the blocks they replace, but this is likely to be fragile:
    • Any changes to either the original blocks or their dynamic replacements could break compatibility.
    • Using types defined in Blockly's blocks/*.ts modules in the dynamic blocks could help (it could help prevent breakages when modifying the plugin, and later help us to notice breakages caused by modifications to the library blocks / generators when upgrading the blockly package in the blockly-samples repo) but most of the relevant types are not currently exported from the top-level blockly/blocks entrypoint.
  • It would therefore probably be better to provide separate block generator functions for the dynamic blocks.
  • We should provide a function to install these as well; this would probably be best done as part of upgrading the whole package to adhere to our current guidelines for publishing block plugins.

Thanks to forum user Martynas Brazauskas for reporting this issue.

@cpcallen cpcallen added type: bug Something isn't working triage labels Aug 8, 2024
@cpcallen cpcallen added this to the Upcoming milestone Aug 9, 2024
@cpcallen cpcallen removed the triage label Aug 9, 2024
@BeksOmega
Copy link
Contributor

I think that ideally these should conform to the same interfaces as the blocks in core, since we advertise them as drop-in replacements. (We ran into similar problems with them not serializing the same way that the blocks in core do). Having them conform to the same interface would allow folks writing generators for new languages to not have to worry about supporting this plugin as well.

@joeftiger
Copy link

I am running into the same problem and switched back to the standard lists_create_with.
The dynamic list is really amazing ergonomically otherwise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants