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

Improving module augmentation #13565

Closed
filipsobol opened this issue Feb 27, 2023 · 3 comments
Closed

Improving module augmentation #13565

filipsobol opened this issue Feb 27, 2023 · 3 comments
Assignees
Labels
squad:core Issue to be handled by the Core team. type:improvement This issue reports a possible enhancement of an existing feature.

Comments

@filipsobol
Copy link
Member

We need to improve how we deal with module augmentation (explained in more details here), to:

  • ensure that module augmentation kicks in when importing individual plugins from packages,
  • all types are available in tests,
  • avoiding arbitrary import './config' imports.

To encourage best practices and ensure that the .d.ts file with module augmentation is always imported, we will require importing all package contents from the main index file instead from individual files in the built package, eg.:

// ✔️
import { Table } from '@ckeditor/ckeditor5-table';

// ❌
import Table from '@ckeditor/ckeditor5-table/src/table';
@filipsobol filipsobol added the type:improvement This issue reports a possible enhancement of an existing feature. label Feb 27, 2023
@filipsobol filipsobol self-assigned this Feb 27, 2023
@CKEditorBot CKEditorBot added the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label Feb 27, 2023
@filipsobol filipsobol added the squad:core Issue to be handled by the Core team. label Feb 28, 2023
@filipsobol
Copy link
Member Author

filipsobol commented Feb 28, 2023

Checklist for migrating packages to the new structure:

  • Create src/augmentation.ts file.
  • Search for all declare module '@ckeditor/ckeditor5-core' declarations and move them to the src/augmentation.ts as done in this file.
  • Remove all import '../xxxconfig' imports.
  • Add import './augmentation'; to the very bottom of the main index.ts.
  • Replace imports in ckeditor5-build-* packages referencing individual files in the src of the package to use the package name instead:
// BEFORE
import Table from '@ckeditor/ckeditor5-table/src/table';

// AFTER
import { Table } from '@ckeditor/ckeditor5-table';

@filipsobol
Copy link
Member Author

filipsobol commented Mar 2, 2023

Below is a summary of the changes introduced in PR #13568 to fix module augmentation and testing within monorepo. The same changes (according to the checklist above) will be applied to all packages using module augmentation.

Module augmentation not being loaded in all cases

Previously, module augmentation were spread across many files and not visible in some other files. To mitigate this, we added import 'xxxconfig'; declarations to load files with needed augmentations. However, because the imports in the source .ts files and the output .d.ts files are different, the code that worked fine during the development didn't always work when the package was built and released. 

To mitigate this, we created an augmentation.ts file with augmentations that cover all the needs during the development. Since it's also imported in the main index.ts file, it will always be loaded as long as code from this package is not loaded from the src folder, e.g:

// BEFORE
import Table from '@ckeditor/ckeditor5-table/src/table';

// AFTER
import { Table } from '@ckeditor/ckeditor5-table';

Module augmentation in tests

We decided NOT to update import paths in tests, because tests often import internals that we don't want to export in the main index.ts. However, this caused TypeScript errors.

To mitigate this, we added "./packages/*/src/augmentation.ts" and "./external/*/packages/*/src/augmentation.ts" to tsconfig.test.json to force TypeScript to always load all available augmentation files when running the tests.
 

Fix loading of tsconfig.json in manual and automated tests

While investigating issues with the tests, we noticed that it loads seemingly arbitrary tsconfig.json files. Once this issue was fixed, we no longer needed to have tsconfig.json (used for testing and loaded by the IDE) and tsconfig.release.json (used for building) for each package. Since tests are always run from the root-level of the whole project, we now have tsconfig.test.json file used specifically for this purpose, and each package now has only have a single tsconfig.json file used by the IDE and for building.

niegowski added a commit that referenced this issue Mar 3, 2023
…mentation-for-table-plugin

Internal: Improve TypeScript configuration in the project and module augmentation in `table` plugin. Related to #13565.
@Reinmar
Copy link
Member

Reinmar commented Mar 6, 2023

Status page of porting all the packages to the new format: https://chill-saturn-f1f.notion.site/Augmentation-migration-62971d5b535e4f7c982fe78d2acd3f0e

filipsobol added a commit that referenced this issue Mar 6, 2023
…mentation-for-essentials-and-more

Internal: Improve TypeScript module augmentation. See #13565.
filipsobol added a commit that referenced this issue Mar 6, 2023
…mentation-for-packages

Internal: Improve TypeScript module augmentation. See #13565.
filipsobol added a commit that referenced this issue Mar 6, 2023
…umentation-3

Other: Improve augmentation in some packages. Related to #13565.
filipsobol added a commit that referenced this issue Mar 7, 2023
…mentation-for-image-plugin

Internal (image): Improve module augmentation in image plugin. Related to #13565.
filipsobol added a commit that referenced this issue Mar 7, 2023
…mentation-for-list-plugin

Internal (list): Improve module augmentation in `list` plugin. Related to #13565.
filipsobol added a commit that referenced this issue Mar 7, 2023
…ntation-20230306

Internal: Improve module augmentation in numerous packages. Related to #13565.
filipsobol added a commit that referenced this issue Mar 7, 2023
…ntation-20230307

Internal: Improve module augmentation in numerous packages. Related to #13565.
filipsobol added a commit that referenced this issue Mar 7, 2023
…umentation-4

Internal: Improve TypeScript module augmentation. Related to #13565.
pomek added a commit that referenced this issue Mar 8, 2023
Internal: Fix `ts-loader` configs used in scripts. Related to #13565.
@arkflpc arkflpc closed this as completed Mar 9, 2023
@CKEditorBot CKEditorBot removed the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label Mar 9, 2023
@Reinmar Reinmar added this to the iteration 61 milestone Mar 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
squad:core Issue to be handled by the Core team. type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

No branches or pull requests

6 participants