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

fix!: cleanup and re-organize npm library #389

Merged
merged 29 commits into from
Apr 15, 2024

Conversation

beawar
Copy link
Contributor

@beawar beawar commented Mar 15, 2024

The goal of this PR is to align the types of the npm package with the objects injected at runtime.

To do so, all the runtime objects are exported in a specific entry point for the library (src/lib.ts) and the declarations of the types are created by typescript itself, removing the need to keeping them updated manually.
The match between the library objects and the runtime ones is double checked by making the runtime injection satisfies the type of the library itself. In this way, a compilation error will occur if there is a difference between the two.

This is a first step to remove the need of mocking the shell entirely when testing the modules. There are still some open points which are preventing this:

  • workers are created using import.meta syntax, which is not compatible with current ts configs
  • notification.mp3 asset is missing inside the library: a quick fix could be including all the assets or only it, but I need more time to investigate the best approach
  • all functions that are app-dependant are at the moment declarations only, and their source code is not packed in the library. We need to understand how to handle them.

Breaking changes

Even if most of the changes represents a refactor, and should be compatible with the previous version, there are some points that will break projects.

AppSetters type is no longer exported

Replace it with the construct typeof <function>

types folder is no longer in the library

If you have some import from subpath of the shell module, remove it and use the root import only. E.g.

// this kind of import is no longer supported
import { something } from '@zextras/carbonio-shell-ui/types/and/other';

// replace it with
import { something } from '@zextras/carbonio-shell-ui';

Declarations of inexisting code are removed

Inside manually created types, there were some declarations of some objects, fields or functions which did not exist in the source code. These declarations are now no longer there. Their usage will now cause a compilation error, preventing unwanted bugs on runtime.

Refactor JSNS enum to an object and improve network types

Soap types have been a little improved, and the type of the _jsns field has been further restricted to always match the Namespace type. Since generic strings can no longer be assigned to this field, the object JSNS, which was previously wrongly exported as an enum inside declarations, is now available as an object and can be used to value the field.

Other changes

  • Update main and types inside the package.json to reflect the structure of the library, and the scripts to build both the production package and the library.
  • Introduce api-extractor to keep track of the api model. It should reduce the risk of releasing breaking change, since any change to it will probably represent a breaking change for the library.
  • To organize better imports and avoid dependency cycles, introduce a couple of rules in eslint, which will split and distinguish all import of the types.
  • Deprecate all constants that are not part of shell, and add a note to export to module only those constants which are useful, not the internal ones.
  • Improve some internal types, and refactor some constant to use lowercase fields inside objects.
  • Export and use JSNS for request namespaces, and improve Soap types.
  • Remove some dead code.
  • Remove some ts-ignore and fix the affected code.

BREAKING-CHANGE: Replace entirely the declarations of the library. See #389 for the list of changes.
Refs: SHELL-157

@beawar beawar requested review from a team as code owners March 15, 2024 16:19

This comment has been minimized.

@beawar
Copy link
Contributor Author

beawar commented Mar 15, 2024

  • Requires: zextras/jenkins-zapp-lib#13

This comment has been minimized.

1 similar comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link

Failed

  • 0.00% Coverage on New Code (is less than 80.00%)
  • 7.91% Duplicated Lines (%) on New Code (is greater than 3.00%)
  • 4 New Issues (is greater than 0)

Analysis Details

4 Issues

  • Bug 0 Bugs
  • Vulnerability 0 Vulnerabilities
  • Code Smell 4 Code Smells

Coverage and Duplications

  • Coverage 0.00% Coverage (0.00% Estimated after merge)
  • Duplications 7.91% Duplicated Code (6.70% Estimated after merge)

Project ID: carbonio-shell-ui

View in SonarQube

@beawar beawar self-assigned this Mar 29, 2024
Copy link

Failed

  • 33.60% Coverage on New Code (is less than 80.00%)
  • 7.90% Duplicated Lines (%) on New Code (is greater than 3.00%)
  • 4 New Issues (is greater than 0)

Analysis Details

4 Issues

  • Bug 0 Bugs
  • Vulnerability 0 Vulnerabilities
  • Code Smell 4 Code Smells

Coverage and Duplications

  • Coverage 33.60% Coverage (59.70% Estimated after merge)
  • Duplications 7.90% Duplicated Code (6.60% Estimated after merge)

Project ID: carbonio-shell-ui

View in SonarQube

1 similar comment
Copy link

Failed

  • 33.60% Coverage on New Code (is less than 80.00%)
  • 7.90% Duplicated Lines (%) on New Code (is greater than 3.00%)
  • 4 New Issues (is greater than 0)

Analysis Details

4 Issues

  • Bug 0 Bugs
  • Vulnerability 0 Vulnerabilities
  • Code Smell 4 Code Smells

Coverage and Duplications

  • Coverage 33.60% Coverage (59.70% Estimated after merge)
  • Duplications 7.90% Duplicated Code (6.60% Estimated after merge)

Project ID: carbonio-shell-ui

View in SonarQube

@CataldoMazzilli CataldoMazzilli self-requested a review April 12, 2024 12:52
nubsthead
nubsthead previously approved these changes Apr 15, 2024
@beawar beawar dismissed stale reviews from nubsthead and CataldoMazzilli via 79278d0 April 15, 2024 12:35
@beawar beawar requested review from nubsthead, CataldoMazzilli and a team and removed request for AliceTincani and MatteoPerdon April 15, 2024 12:35
Copy link

Failed

  • 33.60% Coverage on New Code (is less than 80.00%)
  • 7.90% Duplicated Lines (%) on New Code (is greater than 3.00%)
  • 4 New Issues (is greater than 0)

Analysis Details

4 Issues

  • Bug 0 Bugs
  • Vulnerability 0 Vulnerabilities
  • Code Smell 4 Code Smells

Coverage and Duplications

  • Coverage 33.60% Coverage (59.70% Estimated after merge)
  • Duplications 7.90% Duplicated Code (6.60% Estimated after merge)

Project ID: carbonio-shell-ui

View in SonarQube

Copy link

Failed

  • 33.60% Coverage on New Code (is less than 80.00%)
  • 7.90% Duplicated Lines (%) on New Code (is greater than 3.00%)
  • 4 New Issues (is greater than 0)

Analysis Details

4 Issues

  • Bug 0 Bugs
  • Vulnerability 0 Vulnerabilities
  • Code Smell 4 Code Smells

Coverage and Duplications

  • Coverage 33.60% Coverage (59.70% Estimated after merge)
  • Duplications 7.90% Duplicated Code (6.60% Estimated after merge)

Project ID: carbonio-shell-ui

View in SonarQube

@beawar beawar merged commit dd59b1e into devel Apr 15, 2024
3 of 4 checks passed
@beawar beawar deleted the SHELL-157-cleanup-and-reorganize-types-api-extractor branch April 15, 2024 14:15
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