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

Type of Editor.ui not resolvable with Yarn (strict) PnP mode #17213

Closed
jnoordsij opened this issue Oct 3, 2024 · 5 comments · Fixed by #17790
Closed

Type of Editor.ui not resolvable with Yarn (strict) PnP mode #17213

jnoordsij opened this issue Oct 3, 2024 · 5 comments · Fixed by #17790
Assignees
Labels
resolution:resolved This issue was already resolved (e.g. by another ticket). type:bug This issue reports a buggy (incorrect) behavior.
Milestone

Comments

@jnoordsij
Copy link

jnoordsij commented Oct 3, 2024

📝 Provide detailed reproduction steps (if any)

  1. Setup a project with TypeScript + Yarn PnP (default strict mode)
  2. Create a plugin file along the lines of
import {Plugin} from 'ckeditor5';

export default class FooUI extends Plugin {
  init() {
    this.editor.ui.componentFactory.add('addFoo', (locale) => {
      ...
  }
}
  1. Run tsc to check types

✔️ Expected result

The type of this.editor.ui to be resolved correctly, so that consequently the type of locale in the componentFactory.add call can be inferred.

❌ Actual result

The type of this.editor.ui.componentFactory is marked as any yielding an error Parameter 'locale' implicitly has an 'any' type.ts(7006).

❓ Possible solution

This issue only appears with Yarn PnP mode, as the import import type { EditorUI } from '@ckeditor/ckeditor5-ui'; within the ckeditor5-core cannot be resolved, since the package is not listed as dependency in ckeditor5-core. To make this resolvable, the @ckeditor/ckeditor5-ui should be listed as dependency, peerDependency or optional peerDependency in the ckeditor5-core, with the last one probably being the best option.

Manual workarounds:

  • import the type EditorUI and manually cast with (this.editor.ui as EditorUI)
  • use pnpMode: loose or non-pnp mode with Yarn
  • patch the (optional) peer dependency within your own project by adding the following to .yarnrc.yml:
packageExtensions:
  # See https://github.com/ckeditor/ckeditor5/issues/17213
  "@ckeditor/ckeditor5-core@*":
    peerDependencies:
      "@ckeditor/ckeditor5-ui": "*"
    peerDependenciesMeta:
      "@ckeditor/ckeditor5-ui":
        optional: true

📃 Other details

  • Browser: N/A
  • OS: N/A
  • First affected CKEditor version: 42.0.0 (possibly earlier); edit: still observed on 43.2.0
  • Installed CKEditor plugins: N/A

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

@jnoordsij jnoordsij added the type:bug This issue reports a buggy (incorrect) behavior. label Oct 3, 2024
@Witoso
Copy link
Member

Witoso commented Oct 3, 2024

Resolved in v42.0.2, please verify :)

@Witoso Witoso added the pending:feedback This issue is blocked by necessary feedback. label Oct 3, 2024
@jnoordsij
Copy link
Author

I'm currently running into this issue on the latest version (43.2.0).

@filipsobol filipsobol self-assigned this Oct 3, 2024
@CKEditorBot CKEditorBot removed the pending:feedback This issue is blocked by necessary feedback. label Oct 3, 2024
@filipsobol
Copy link
Member

We use a linter that checks for missing dependencies and devDependencies. At some point, we decided that it should treat type-only imports as devDependencies, but we will review this decision.

@jnoordsij
Copy link
Author

jnoordsij commented Oct 23, 2024

As an alternative to dependencies vs devDependencies, adding it as (optional) peerDependencies entry will also work I found (added the configuration snippet in workarounds above). Maybe this might be a more suitable middle ground for type-only imports?

@CKEditorBot CKEditorBot added the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label Jan 8, 2025
filipsobol added a commit to ckeditor/ckeditor5-dev that referenced this issue Jan 20, 2025
Internal: Update `eslint-config-ckeditor5` to `^9.0.0`.

Feature (dependency-checker): Treat type dependencies as production dependencies. Related to ckeditor/ckeditor5#17213.

MINOR BREAKING CHANGE (dependency-checker): Treat type dependencies as production dependencies.
filipsobol added a commit that referenced this issue Jan 22, 2025
Fix: Treat types as production dependencies. Fixes #17213.
@CKEditorBot CKEditorBot removed the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label Jan 22, 2025
@filipsobol
Copy link
Member

The fixes are merged and will be available in the next release.

@CKEditorBot CKEditorBot added this to the iteration 82 milestone Jan 22, 2025
@filipsobol filipsobol added the resolution:resolved This issue was already resolved (e.g. by another ticket). label Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
resolution:resolved This issue was already resolved (e.g. by another ticket). type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
4 participants