-
Notifications
You must be signed in to change notification settings - Fork 661
Description
Check for duplicates
- I have searched for similar issues before opening a new one.
Component
dev-tools kinda, but actually all of our plugins.
Description
If you update the required version of Blockly in a plugin to be higher than the version of Blockly required by dev-tools, you end up with two Blockly dependencies that are not deduped.
For example, if dev-tools requires ^10.0.0 and field-colour requires ^10.1.3 you get:
$ npm ls blockly
@blockly/field-colour@3.0.4 /usr/local/google/home/bwestberg/Dev/blockly-samples/plugins/field-colour
├─┬ @blockly/dev-scripts@2.0.1 -> ./../dev-scripts
│ ├─┬ @blockly/eslint-config@3.0.0 -> ./../eslint-config
│ │ └── blockly@10.0.0
│ └── blockly@10.0.0
├─┬ @blockly/dev-tools@7.0.3 -> ./../dev-tools
│ ├─┬ @blockly/block-test@5.0.0 -> ./../block-test
│ │ ├─┬ @blockly/dev-scripts@2.0.1 -> ./../dev-scripts
│ │ │ └── blockly@10.0.0 deduped
│ │ └── blockly@10.0.0
│ ├─┬ @blockly/dev-scripts@2.0.1 -> ./../dev-scripts
│ │ └── blockly@10.0.0 deduped
│ ├─┬ @blockly/theme-dark@6.0.1 -> ./../theme-dark
│ │ ├─┬ @blockly/dev-scripts@2.0.1 -> ./../dev-scripts
│ │ │ └── blockly@10.0.0 deduped
│ │ └── blockly@10.0.0
│ ├─┬ @blockly/theme-deuteranopia@5.0.1 -> ./../theme-deuteranopia
│ │ ├─┬ @blockly/dev-scripts@2.0.1 -> ./../dev-scripts
│ │ │ └── blockly@10.0.0 deduped
│ │ └── blockly@10.0.0
│ ├─┬ @blockly/theme-highcontrast@5.0.1 -> ./../theme-highcontrast
│ │ ├─┬ @blockly/dev-scripts@2.0.1 -> ./../dev-scripts
│ │ │ └── blockly@10.0.0 deduped
│ │ └── blockly@10.0.0
│ ├─┬ @blockly/theme-tritanopia@5.0.1 -> ./../theme-tritanopia
│ │ ├─┬ @blockly/dev-scripts@2.0.1 -> ./../dev-scripts
│ │ │ └── blockly@10.0.0 deduped
│ │ └── blockly@10.0.0
│ └── blockly@10.0.0
└── blockly@10.1.3
The last two lines are the important ones:
│ └── blockly@10.0.0
└── blockly@10.1.3
This doesn't cause any functional problems for JavaScript plugins, because dev-tools is actually compatible with ^10.1.3 because 10.0.0 <= 10.1.3 < 11.x.x which is what ^10.0.0 specifies.
But it does cause problems for TypeScript plugins, because when you compile them you will get type conflicts between the newer Blockly types and the older Blockly types. For example:
ERROR in /usr/local/google/home/bwestberg/Dev/blockly-samples/plugins/field-colour/test/index.ts(52,35)
TS2345: Argument of type '(blocklyDiv: HTMLElement, options: BlocklyOptions) => WorkspaceSvg' is not assignable to parameter of type '(blocklyDiv: HTMLElement, options: BlocklyOptions) => Workspace'.
Types of parameters 'options' and 'options' are incompatible.
Type 'import("/usr/local/google/home/bwestberg/Dev/blockly-samples/plugins/dev-tools/node_modules/blockly/core/blockly_options").BlocklyOptions' is not assignable to type 'import("/usr/local/google/home/bwestberg/Dev/blockly-samples/plugins/field-colour/node_modules/blockly/core/blockly_options").BlocklyOptions'.
Types of property 'parentWorkspace' are incompatible.
Type 'import("/usr/local/google/home/bwestberg/Dev/blockly-samples/plugins/dev-tools/node_modules/blockly/core/workspace_svg").WorkspaceSvg | undefined' is not assignable to type 'import("/usr/local/google/home/bwestberg/Dev/blockly-samples/plugins/field-colour/node_modules/blockly/core/workspace_svg").WorkspaceSvg | undefined'.
Property 'getSvgGroup' is missing in type 'import("/usr/local/google/home/bwestberg/Dev/blockly-samples/plugins/dev-tools/node_modules/blockly/core/workspace_svg").WorkspaceSvg' but required in type 'import("/usr/local/google/home/bwestberg/Dev/blockly-samples/plugins/field-colour/node_modules/blockly/core/workspace_svg").WorkspaceSvg'.
Reproduction steps
- Bump the version of Blockly in a typescript plugin without bumping dev-tools.
- Run
npm run startto see version conflicts.
Suggested solution
We want to update all of our typescript plugins so that they use /their/ version of Blockly for compilation instead of whatever dev tools is using. We can do that by adding paths to our TSConfig:
{
"compilerOptions": {
// Other options etc...
"paths": {
"blockly/*": ["node_modules/blockly/*"]
}
},
// Other options etc...
}
Hoisting
If we ever add hoisting of dependencies, this will break, so we should remove it.