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

Docs issue report from "getting-started/setup/optimizing-build-size.html" #16684

Closed
hendeltom opened this issue Jul 8, 2024 · 5 comments
Closed
Labels
resolution:resolved This issue was already resolved (e.g. by another ticket). squad:core Issue to be handled by the Core team. type:docs This issue reports a task related to documentation (e.g. an idea for a guide).

Comments

@hendeltom
Copy link

Origin URL

https://ckeditor.com/docs/ckeditor5/latest/getting-started/setup/optimizing-build-size.html

Project version

42.0.0

Is the information outdated? How?

The mentioned import paths do not work with Typescript. See below.

Is there something missing in the guide? What is it?

The example code

- import { ClassicEditor, Bold, Italic and Table } from 'ckeditor5';

+ import { ClassicEditor } from '@ckeditor/ckeditor5-editor-classic/dist/index.js';
+ import { Bold, Italic } from '@ckeditor/ckeditor5-basic-styles/dist/index.js';
+ import { Table } from '@ckeditor/ckeditor5-table/dist/index.js';

does not work for Typescript builds because the ./dist folders of all plugins do not contains the file index.d.ts as the ./src folders do. So for the moment, then only import paths that would work are

import { ClassicEditor } from '@ckeditor/ckeditor5-editor-classic/src/index';
import { Bold, Italic } from '@ckeditor/ckeditor5-basic-styles/src/index';
import { Table } from '@ckeditor/ckeditor5-table/src/index';

Is there anything else you would like to add?

No response

User agent

Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126.0.0.0 Safari/537.36

@hendeltom hendeltom added squad:core Issue to be handled by the Core team. type:docs This issue reports a task related to documentation (e.g. an idea for a guide). labels Jul 8, 2024
@Reinmar
Copy link
Member

Reinmar commented Jul 8, 2024

The types are there:

But TS indeed cannot find them:

➜  my-app yarn build
yarn run v1.22.21
$ tsc && vite build
src/main.ts:3:31 - error TS7016: Could not find a declaration file for module '@ckeditor/ckeditor5-editor-classic/dist/index.js'. '/Users/p/Workspace/my-app/node_modules/@ckeditor/ckeditor5-editor-classic/dist/index.js' implicitly has an 'any' type.
  If the '@ckeditor/ckeditor5-editor-classic' package actually exposes this module, try adding a new declaration (.d.ts) file containing `declare module '@ckeditor/ckeditor5-editor-classic/dist/index.js';`

3 import { ClassicEditor } from '@ckeditor/ckeditor5-editor-classic/dist/index.js';

@Reinmar
Copy link
Member

Reinmar commented Jul 8, 2024

cc @filipsobol

@filipsobol
Copy link
Member

filipsobol commented Jul 10, 2024

The problem is that these types are in a different folder than the code (dist/* vs dist/types/*). This could be fixed by either (1) moving the types to the same folder as the code or (2) by using the exports field in the package.json.

Option 1 is easier to implement, but it would introduce a lot of noise into the dist folder, making it harder to find index.js, index.css, index-content.css, and index-editor.css.

Option 2 requires much more work, as it requires adding an exports field to the package.json file in all packages. Additionally, if the exports field is declared, then anything not registered there is NOT accessible from JS, so we would have to be very careful not to block access to styles, translations, build, src, theme folders or the package.json itself. It's not very complex, but given the amount of variables something can definitely go wrong.

@Witoso
Copy link
Member

Witoso commented Jul 16, 2024

To keep it KISS I would vote for option 1. Do we know how many files on average would appear?

filipsobol added a commit that referenced this issue Jul 22, 2024
…prod

Fix: Add dependencies used in the new `dist` folder as production `dependencies` instead of `devDependencies`. Related to #16646.

Fix (ckeditor5): Change the path to the types in the `package.json`. See #16684.

Internal: Update `dev-*` packages.
pszczesniak added a commit to ckeditor/ckeditor5-package-generator that referenced this issue Jul 22, 2024
Fix (generator): Change the path to the types in the `package.json`  in `ts` templates. See ckeditor/ckeditor5#16684.
@filipsobol
Copy link
Member

filipsobol commented Jul 25, 2024

This problem has been fixed in 42.0.2.

@Witoso Witoso added the resolution:resolved This issue was already resolved (e.g. by another ticket). label Jul 26, 2024
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). squad:core Issue to be handled by the Core team. type:docs This issue reports a task related to documentation (e.g. an idea for a guide).
Projects
None yet
Development

No branches or pull requests

4 participants