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

Client code refactoring #7208

Merged
merged 6 commits into from
Dec 4, 2023
Merged

Client code refactoring #7208

merged 6 commits into from
Dec 4, 2023

Conversation

bsekachev
Copy link
Member

@bsekachev bsekachev commented Nov 30, 2023

Motivation and context

  • I converted (cvat-data, cvat-core, cvat-canvas, cvat-canvas3d) to ESM. In fact they already were ESM, but still considered as CommonJS modules without explicit { "type": "module" } in package.json.
  • CommonJS files renamed from .js to .cjs within ESM modules (it is a part of specification)
  • I added cvat-core global types, now getCore() returns typed object
  • Updated browserlist config, Firefox < 102 is not used on the market according to browserlist info. I would like also update spec for Chrome for more modern version, but these old version, like 63, 64, 65 are still used by users around the world :(
  • Updated many eslint packages, minor code refactoring related to these changes, updated linting rules
  • And actually many other packages also were updated
  • A little bit refactored jest tests, I tried to minimize using window there. I hope soon we will be able to get rid of jsdom dependency. Right now 3rdparty packages from cvat-data don't allow to do that
  • Removed extra code from cvat-data module.
  • Supported typescript workers with Webpack 5, removed worker loader
  • Removed events.subscribe, events.unsubscribe (we did not even have implementation for these cvat-core methods)

How has this been tested?

Checklist

  • I submit my changes into the develop branch
  • I have created a changelog fragment
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • I have linked related issues (see GitHub docs)
  • I have increased versions of npm packages if it is necessary
    (cvat-canvas,
    cvat-core,
    cvat-data and
    cvat-ui)

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.

@bsekachev bsekachev mentioned this pull request Nov 30, 2023
14 tasks
Copy link

codecov bot commented Nov 30, 2023

Codecov Report

Merging #7208 (19be452) into develop (cc10cb9) will increase coverage by 0.01%.
Report is 4 commits behind head on develop.
The diff coverage is 90.95%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #7208      +/-   ##
===========================================
+ Coverage    81.48%   81.50%   +0.01%     
===========================================
  Files          365      365              
  Lines        39918    39257     -661     
  Branches      3703     3631      -72     
===========================================
- Hits         32529    31996     -533     
+ Misses        7389     7261     -128     
Components Coverage Δ
cvat-ui 75.32% <90.95%> (-0.18%) ⬇️
cvat-server 87.09% <ø> (-0.01%) ⬇️

cvat-core/src/annotations-objects.ts Show resolved Hide resolved
cvat-core/src/api-implementation.ts Show resolved Hide resolved
cvat-core/src/download.worker.ts Outdated Show resolved Hide resolved
cvat-core/src/index.ts Show resolved Hide resolved
cvat-data/src/ts/3rdparty/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you delete this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because we did not use it

Copy link
Member Author

Choose a reason for hiding this comment

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

  • Initially we used this patch file to patch the library during build
  • As I discovered, patched version was already commited long time ago and we don't use the file anymore
  • Finally I do not really see a reason to patch this file, their repository is dead, we don't receive any updates. License allows us to modify it, so..

@bsekachev bsekachev merged commit 123c394 into develop Dec 4, 2023
34 checks passed
@bsekachev bsekachev deleted the bs/refactoring branch December 4, 2023 17:35
amjadsaadeh pushed a commit to amjadsaadeh/cvat that referenced this pull request Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants