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.

Sign up for GitHub

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

Jump to bottom

fix: load only specified typings for TypeScript #14808

Merged
merged 6 commits into from
Aug 25, 2023
Merged

fix: load only specified typings for TypeScript #14808

merged 6 commits into from
Aug 25, 2023

Conversation

filipsobol
Copy link
Member

@filipsobol filipsobol commented Aug 17, 2023 •
edited
Loading

Suggested merge commit message (convention)

Fix: Don't rely on global object available only in Node. Fixes ckeditor/vite-plugin-ckeditor5#17 and #14801.

Internal: Load only specified typings for TypeScript. Closes #14173.

@filipsobol filipsobol mentioned this pull request Aug 17, 2023
Closed
@filipsobol filipsobol mentioned this pull request Aug 21, 2023
Closed
arkflpc
arkflpc approved these changes Aug 25, 2023
View reviewed changes
Copy link
Contributor

@arkflpc arkflpc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. However, I'd seek approval from @ckeditor/ckeditor-5-devops team as well.

@filipsobol filipsobol linked an issue Aug 25, 2023 that may be closed by this pull request
Missing name global when scrolling ckeditor - header does not stick #14801
Closed
pomek
pomek approved these changes Aug 25, 2023
View reviewed changes
Copy link
Member

@pomek pomek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

I like the proposed empty array as it still loads needed types, but a developer does not have to remember to add new types when using some external library.

@pomek
Copy link
Member

pomek commented Aug 25, 2023

I believe the proposed message should be Internal, or if it really matters for integrators, Other.

@filipsobol
Copy link
Member Author

I believe the proposed message should be Internal, or if it really matters for integrators, Other.

In the packages/ckeditor5-utils/src/dom/getelementsintersectionrect.ts file we're fixing code error, so I believe that Fix is appropriate.

@filipsobol filipsobol merged commit b7a984b into master Aug 25, 2023
@filipsobol filipsobol deleted the ck/14173-load-only-specified-typings-for-typescript branch August 25, 2023 12:12
@filipsobol filipsobol mentioned this pull request Aug 25, 2023
Closed
This was referenced Aug 29, 2023
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arkflpc arkflpc arkflpc approved these changes

@pomek pomek pomek approved these changes

@niegowski niegowski Awaiting requested review from niegowski

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@filipsobol @pomek @arkflpc