ckeditor5 icon indicating copy to clipboard operation
ckeditor5 copied to clipboard

Consider removing `global` from `utils` package

Open filipsobol opened this issue 2 years ago • 2 comments

I noticed that the utils package exports a variable called global, which is used to access window and document. Comments suggested that this was a hack to mock window and document in sinon.

After some testing it turns out that this is may no longer be needed, because direct access to window and document works just fine, so I've created two more PRs (https://github.com/ckeditor/ckeditor5/pull/14818 and https://github.com/cksource/ckeditor5-commercial/pull/5461) to remove global from utils.

Since this is a breaking change, if we consider to remove this variable, we'll need to (1) write an ADR to get feedback from the community and other teams.

filipsobol avatar Aug 24 '23 11:08 filipsobol

Concern raised by @dpawlowski-cksource in https://github.com/ckeditor/ckeditor5/issues/14173#issuecomment-1687843450:

Hello guys, thanks to @LukaszGudel we noticed that you plan to remove global usage from ckeditor5-utils. We are using it in cs-client in few files e.g. here.

As we are using cs-client in our E2E tests (running constantly every 5 min) that are launched in Node.js (which doesn't have access to DOM elements such as window, document) so far it was enough to use this trick.

However, we are not sure if this will still work after removing global completely.

Are you able to release for us some test version of ckeditor5 that we could use in cs-client to test whether we will be able to use it in our E2E tests (Node.js) after your changes?


It would also be ideal to get an editorBundle that we can test to see if it works with features like documents-storage. However, as these functions are already run in the browser (puppeteer), we do not anticipate problems with this.

filipsobol avatar Aug 24 '23 11:08 filipsobol

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may still be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

CKEditorBot avatar Aug 25 '24 01:08 CKEditorBot

We've closed your issue due to inactivity. We understand that the issue may still be relevant. If so, feel free to open a new one (and link this issue to it).

CKEditorBot avatar Oct 01 '24 23:10 CKEditorBot