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

Verify the dependency checker #8817

Closed
psmyrek opened this issue Jan 12, 2021 · 3 comments · Fixed by ckeditor/ckeditor5-dev#687
Closed

Verify the dependency checker #8817

psmyrek opened this issue Jan 12, 2021 · 3 comments · Fixed by ckeditor/ckeditor5-dev#687
Assignees
Labels
domain:extending-builds squad:platform Issue to be handled by the Platform team. type:task This issue reports a chore (non-production change) and other types of "todos".

Comments

@psmyrek
Copy link
Contributor

psmyrek commented Jan 12, 2021

Provide a description of the task

The dependency checker allows to import ComponentFactory from '@ckeditor/ckeditor5-ui/src/componentfactory'; in ckeditor5-core package (packages/ckeditor5-core/src/editor/editorui.js), despite lack of @ckeditor/ckeditor5-ui in ckeditor5-core's dependencies. Verify why.

Additionally:

  • check that it would tell you, that your DLL package must have ckeditor5 in the dependencies, if you imported something from ckeditor5/....
  • check that it would tell you, that your non-DLL package has @ckeditor/ckeditor5-* in the dependencies.
  • add it to husky so it's run locally on commit.
  • take the occasion to rethink this tool's logic (check out with @ma2ciek why do they have a separate dependency checker in CF - maybe it's a good moment to unify them).
@psmyrek psmyrek added type:task This issue reports a chore (non-production change) and other types of "todos". squad:platform Issue to be handled by the Platform team. labels Jan 12, 2021
@psmyrek psmyrek self-assigned this Jan 12, 2021
@psmyrek
Copy link
Contributor Author

psmyrek commented Jan 13, 2021

The dependency checker verifies, if a package included in the code is missing in package.json, but it currently does not verify, if a package already listed as dependencies or devDependencies should belong to that list: i.e. all packages in devDependencies should be there if they are used only in tests, but if any of it is imported in the src/**/*.js then this particular package should be defined in dependencies. This is the answer for the question, why the dependency checker does not complain about importing something from @ckeditor/ckeditor5-ui in ckeditor5-core despite lack of @ckeditor/ckeditor5-ui in ckeditor5-core's dependencies - it is actually included in devDependencies there!

So I'm adding another point to improve:

  • verify that all packages belong to proper group: dependencies or devDependencies

@psmyrek
Copy link
Contributor Author

psmyrek commented Jan 13, 2021

The dependency checker does not inform about unused packages in dependencies or devDependencies, so:

  • fix detecting unused packages

@psmyrek
Copy link
Contributor Author

psmyrek commented Jan 15, 2021

The dependency checker does not check CSS files properly, so:

  • fix handling CSS files

@Reinmar Reinmar added this to the iteration 39 milestone Jan 18, 2021
pomek added a commit to ckeditor/ckeditor5-dev that referenced this issue Jan 19, 2021
Fix (tests): Improved the dependency checker in detecting missing, unused, or misplaced packages from JS, CSS, and `package.json` files. Closes ckeditor/ckeditor5#8817.
pomek added a commit that referenced this issue Jan 19, 2021
Internal: Fixed unused or misplaced dependencies and added dependency checker to Husky. See: #8817.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:extending-builds squad:platform Issue to be handled by the Platform team. type:task This issue reports a chore (non-production change) and other types of "todos".
Projects
None yet
2 participants