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

Unclear whether we can use imports from index.js files of core (DLL) packages? #10375

Closed
Reinmar opened this issue Aug 18, 2021 · 5 comments · Fixed by ckeditor/eslint-plugin-ckeditor5-rules#7
Assignees
Labels
squad:devops Issue to be handled by the Devops team. type:task This issue reports a chore (non-production change) and other types of "todos".

Comments

@Reinmar
Copy link
Member

Reinmar commented Aug 18, 2021

See:
Originally posted by @Reinmar in #10352 (comment)

The below change does not trigger a linter warning, so it's unclear to me how the code should be written:

image

@Reinmar
Copy link
Member Author

Reinmar commented Aug 18, 2021

@Reinmar Reinmar added squad:devops Issue to be handled by the Devops team. type:task This issue reports a chore (non-production change) and other types of "todos". labels Aug 18, 2021
@pomek
Copy link
Member

pomek commented Aug 18, 2021

Core DLL packages can import between themselves using any import condition.

  • through the ckreditor5 package,
  • an import from a package index.js,
  • a direct import of required thing.

@Reinmar
Copy link
Member Author

Reinmar commented Aug 23, 2021

There's a history behind the inconsistency. The problem is that core DLL packages may want to import something from another core DLL package that is not exposed in index.js (because we didn't want to make it a public API yet). So, at least some imports would have to use the @ckeditor/ckeditor5-.../src/ notation. Hence, all should.

Let's create a rule checking for that. And document the "why" briefly in the documentation introduced in #10051.

Finally, create a followup to check what kind of imports between core DLL packages block us from moving to index.js-based imports. NOTE: To make all imports in our code consistent, we'd need to use ckeditor/src/... notation, not @ckeditor/ckeditor5-foo notation. So, the fragment of code in my first comment was n fact not even consistent with that. See #10375 (comment)

psmyrek added a commit that referenced this issue Sep 3, 2021
Docs: Mention imports between DLLs packages. See #10375.
psmyrek added a commit to ckeditor/eslint-plugin-ckeditor5-rules that referenced this issue Sep 3, 2021
Other: An import between DLL packages must use the full name of the imported package. Closes ckeditor/ckeditor5#10375.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
squad:devops Issue to be handled by the Devops team. type:task This issue reports a chore (non-production change) and other types of "todos".
Projects
None yet
2 participants