Skip to content

Commit

Permalink
chore: fix "Public API Changes" bot (SAP#15883)
Browse files Browse the repository at this point in the history
### Improvements
- make the "Public API changes" bot back working in CI (get rid of all the errors in the script)
  - side effect: reduce the time of executing "Public API changes" bot from 2h back to ~30min (see [test PR](https://github.com/SAP/spartacus/actions/runs/2580268027))
- reduced the warnings noise in logs:
  - disabled API-extractor warnings not relevant in our project, e.g. [warning for missing release tags like alpha/beta/internal/public in our TSDocs](https://api-extractor.com/pages/messages/ae-missing-release-tag/),
  - disabled API-extractor warnings about missing exports, because sometimes caused false-positive warnings e.g. when an interface is exported, but from a different library
  - disabled all warnings from TSDocs parser (checking the syntax of TSDocs is not in scope of our breaking changes detection bot; generated ~30k warnings per script execution before)
- reduced the warnings noise in the report comment:
  - API Extractor warnings are not printed to the report comment, but only in the console of the script - to avoid fake reports because of a different file path, for example:
     ```diff
     -// /github/workspace/branch-clone/dist/<...file path> - <warning message>
     +// /github/workspace/dist/<...file path> - <warning message>
    ```

### Changes
- analyze d.ts instead of TS files in dependent libraries (e.g. when finding `import {X} from '@spartacus/core'`, analyze `dist/core/.../.d.ts` instead of `/projects/core/.../.ts`)
- disable api-extractor warning of type `ae-missing-release-tag` and `ae-forgotten-export`
- disable all TSDoc warnings (of type `tsdocMessageReporting`)
- disable reporting warnings in the markdown report, but leave them in the script console output
- add explicit return type for `APP_INITIALIZER`s that returned `()=>Promise<Config>` - to avoid api-extractor error `ERROR: Failed to fetch entity for import() type node: import("@spartacus/core").Config`
  - by the way, I added also missing return types for other `APP_INITIALIZER`s, which was not essential for this PR
- by the way, removed some redundant JSDoc notices `@member` from `occ-models.ts` - which caused warnings in api-extractor and were redundant

### Implementation notes
Previously for API Extractor tool we used the tsconfig with path mappings to source code (e.g. for resolving an import from `@spartacus/core` it looked up the source code TS files in `projects/core/*`).

According to https://api-extractor.com/pages/messages/ae-wrong-input-file-type/ , it seems that we were using the api-extractor tool not in a way that the tool's authors assumed. We analyzed TS files (only in dependency packages), but api-extractor expects to analyze d.ts

Moreover, since API Extractor v7.24.0 (released 2022-05-14), analyzing TS files throws an error (see [changelog](https://github.com/microsoft/rushstack/blob/main/apps/api-extractor/CHANGELOG.md#7240) and [troubleshooting page](https://api-extractor.com/pages/messages/ae-wrong-input-file-type/)), which caused our CI script for api-extractor to fail since 2022-05-14.

In this PR, we start using the "production" version of `tsconfig` (`tsconfig.app.prod.json` instead of `tsconfig.json`), which uses `"paths"` mapping to the `/dist` folder, which contains d.ts files.
  • Loading branch information
Platonn authored Jun 29, 2022
1 parent f1d48e7 commit 9d9ec4c
Show file tree
Hide file tree
Showing 22 changed files with 506 additions and 1,681 deletions.
17 changes: 14 additions & 3 deletions .github/api-extractor-action/api-extractor.json
Original file line number Diff line number Diff line change
Expand Up @@ -305,8 +305,19 @@
*/
"extractorMessageReporting": {
"default": {
"logLevel": "warning"
// "addToApiReportFile": false
"logLevel": "warning",
"addToApiReportFile": false
},

// Spartaus: currently we don't use the release tags @alpha/@beta/@internal/@public in our JSDocs:
"ae-missing-release-tag": {
"logLevel": "none"
},

// Spartacus: disabled due to false-positive warnings e.g. when an interface is exported, but from a different library
"ae-forgotten-export": {
"logLevel": "none",
"addToApiReportFile": false
}

// "ae-extra-release-tag": {
Expand All @@ -326,7 +337,7 @@
*/
"tsdocMessageReporting": {
"default": {
"logLevel": "warning"
"logLevel": "none"
// "addToApiReportFile": false
}

Expand Down
2 changes: 1 addition & 1 deletion feature-libs/asm/core/asm-core.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { AsmStoreModule } from './store/asm-store.module';

export function asmStatePersistenceFactory(
asmStatePersistenceService: AsmStatePersistenceService
) {
): () => void {
const result = () => asmStatePersistenceService.initSync();
return result;
}
Expand Down
2 changes: 1 addition & 1 deletion feature-libs/asm/root/asm-loader.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export class AsmLoaderModule {}
* to a real component; the router and state aren't available in an optimized
* way during the APP_INITIALIZER.
*/
export function asmFactory(asmEnablerService: AsmEnablerService) {
export function asmFactory(asmEnablerService: AsmEnablerService): () => void {
const isReady = () => {
asmEnablerService.load();
};
Expand Down
8 changes: 6 additions & 2 deletions feature-libs/cart/base/core/cart-persistence.module.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
import { NgModule } from '@angular/core';
import { Action, ActionReducer, MetaReducer, META_REDUCERS } from '@ngrx/store';
import { CartType } from '@spartacus/cart/base/root';
import { ConfigInitializerService, MODULE_INITIALIZER } from '@spartacus/core';
import {
Config,
ConfigInitializerService,
MODULE_INITIALIZER,
} from '@spartacus/core';
import { tap } from 'rxjs/operators';
import { MultiCartStatePersistenceService } from './services/multi-cart-state-persistence.service';

export function cartStatePersistenceFactory(
cartStatePersistenceService: MultiCartStatePersistenceService,
configInit: ConfigInitializerService
) {
): () => Promise<Config> {
const result = () =>
configInit
.getStable('context')
Expand Down
2 changes: 1 addition & 1 deletion feature-libs/smartedit/root/smart-edit-root.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { SmartEditLauncherService } from './services/smart-edit-launcher.service

export function smartEditFactory(
smartEditLauncherService: SmartEditLauncherService
) {
): () => void {
const isReady = () => {
smartEditLauncherService.load();
};
Expand Down
3 changes: 2 additions & 1 deletion integration-libs/cdc/root/cdc-root.module.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { APP_INITIALIZER, NgModule } from '@angular/core';
import {
CmsConfig,
Config,
ConfigInitializerService,
provideDefaultConfigFactory,
} from '@spartacus/core';
Expand All @@ -13,7 +14,7 @@ import { CdcJsService } from './service/cdc-js.service';
export function cdcJsFactory(
cdcJsService: CdcJsService,
configInit: ConfigInitializerService
) {
): () => Promise<Config> {
const func = () =>
configInit
.getStable('context', 'cdc')
Expand Down
2 changes: 1 addition & 1 deletion projects/core/src/auth/user-auth/user-auth.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { AuthStorageService } from './services/auth-storage.service';
export function checkOAuthParamsInUrl(
authService: AuthService,
configInit: ConfigInitializerService
) {
): () => Promise<void> {
const result = () =>
configInit
.getStable()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,28 +1,29 @@
import { LOCATION_INITIALIZED } from '@angular/common';
import {
APP_INITIALIZER,
ModuleWithProviders,
NgModule,
Optional,
} from '@angular/core';
import { ConfigInitializerService } from './config-initializer.service';
import { Config } from '../config-tokens';
import {
ConfigInitializer,
CONFIG_INITIALIZER,
CONFIG_INITIALIZER_FORROOT_GUARD,
ConfigInitializer,
} from './config-initializer';
import { LOCATION_INITIALIZED } from '@angular/common';
import { ConfigInitializerService } from './config-initializer.service';

export function configInitializerFactory(
configInitializer: ConfigInitializerService,
initializers: ConfigInitializer[]
) {
): () => Promise<void> {
const isReady = () => configInitializer.initialize(initializers);
return isReady;
}

export function locationInitializedFactory(
configInitializer: ConfigInitializerService
) {
): Promise<Config> {
return configInitializer.getStable().toPromise();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
export function configValidatorFactory(
configInitializer: ConfigInitializerService,
validators: ConfigValidator[]
) {
): () => void {
const validate = () => {
if (isDevMode()) {
configInitializer
Expand Down
Loading

0 comments on commit 9d9ec4c

Please sign in to comment.