Skip to content

Commit 3378c32

Browse files
authored
fix(type-check): add type check and fix strictNullChecks error (#503)
1 parent a8c40ed commit 3378c32

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

57 files changed

+613
-334
lines changed

.github/workflows/ci.yml

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ on:
99
- main
1010

1111
jobs:
12-
test:
12+
unit-test:
1313
runs-on: ubuntu-20.04
1414
strategy:
1515
matrix:
@@ -49,7 +49,6 @@ jobs:
4949
uses: codecov/codecov-action@v2
5050
with:
5151
directory: ./coverage
52-
5352
e2e-test:
5453
runs-on: ubuntu-20.04
5554
strategy:
@@ -90,3 +89,24 @@ jobs:
9089
# Recommended: pass the GitHub token lets this action correctly
9190
# determine the unique run id necessary to re-run the checks
9291
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
92+
type-check:
93+
runs-on: ubuntu-20.04
94+
strategy:
95+
matrix:
96+
node-version: [16.14.0]
97+
steps:
98+
- uses: actions/checkout@v2
99+
- name: Use Node.js ${{ matrix.node-version }}
100+
uses: actions/setup-node@v2
101+
with:
102+
node-version: ${{ matrix.node-version }}
103+
- uses: pnpm/[email protected]
104+
name: Install pnpm
105+
id: pnpm-install
106+
with:
107+
version: 6.32.17
108+
run_install: false
109+
- name: install dependency
110+
run: pnpm install --filter=\!garfish-docs
111+
- name: check type
112+
run: pnpm type:check

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
"test:coverage": "jest --collect-coverage",
1616
"cy:open": "cypress open",
1717
"cy:run": "cypress run",
18+
"type:check": "tsc -noEmit",
1819
"changeset": "npx changeset",
1920
"reset": "pnpm -r exec -- rm -rf node_modules && pnpm -r exec -- rm -rf dist && rm -rf node_modules",
2021
"setup": "cross-env DISABLE_GARFISH_CHECK_INTERNAL=true pnpm install",

packages/bridge-react-v18/__tests__/reactBridge.spec.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import { reactBridge } from '../src/reactBridge';
66
import '@testing-library/jest-dom';
7+
import assert from 'assert';
78

89
const domElId = '#sub-app-container';
910
const cssSelector = '#app';
@@ -242,7 +243,10 @@ describe('react-bridge', () => {
242243
expect(React.createElement).toHaveBeenCalled();
243244
expect(document.getElementById('test_el')).toBeInTheDocument();
244245
expect(document.getElementById('test_el')).not.toBeNull;
245-
expect(document.getElementById('test_el').children).not.toBeNull;
246+
247+
assert(document.getElementById('test_el'));
248+
expect(document.getElementById('test_el')).not.toBeNull;
249+
expect(document.getElementById('test_el')?.children).not.toBeNull;
246250
expect(document.getElementById('app')).toBeNull;
247251
});
248252

@@ -258,8 +262,10 @@ describe('react-bridge', () => {
258262

259263
lifeCycles.render({ ...appInfo, props });
260264
expect(React.createElement).toHaveBeenCalled();
265+
266+
assert(document.getElementById('app'));
261267
expect(document.getElementById('app')).not.toBeNull;
262-
expect(document.getElementById('app').children).not.toBeNull;
268+
expect(document.getElementById('app')?.children).not.toBeNull;
263269
});
264270
});
265271
});

packages/bridge-react-v18/src/reactBridge.ts

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,13 @@ type Options = UserOptions<
2020

2121
const defaultOpts = {
2222
// required - one or the other or both
23-
rootComponent: null,
24-
loadRootComponent: null,
23+
rootComponent: undefined,
24+
loadRootComponent: undefined,
2525

2626
// optional opts
27-
renderType: null,
28-
errorBoundary: null,
29-
el: null,
27+
renderType: undefined,
28+
errorBoundary: undefined,
29+
el: undefined,
3030
canUpdate: true, // by default, allow parcels created with garfish-react-bridge to be updated
3131
suppressComponentDidCatchWarning: false,
3232
domElements: {},
@@ -139,15 +139,21 @@ function mount(opts: Options, appInfo: PropsInfo) {
139139
opts,
140140
});
141141

142-
opts.domElements[appInfo.appName] = domElement;
143-
opts.renderResults[appInfo.appName] = renderResult;
142+
if (opts.domElements) {
143+
opts.domElements[appInfo.appName] = domElement;
144+
}
145+
if (opts.renderResults) {
146+
opts.renderResults[appInfo.appName] = renderResult;
147+
}
144148
}
145149

146150
function unmount(opts: Options, appInfo: PropsInfo) {
147-
const root = opts.renderResults[appInfo.appName];
148-
root.unmount();
149-
delete opts.domElements[appInfo.appName];
150-
delete opts.renderResults[appInfo.appName];
151+
if (opts.renderResults) {
152+
const root = opts.renderResults[appInfo.appName];
153+
root.unmount();
154+
opts.domElements && delete opts.domElements[appInfo.appName];
155+
delete opts.renderResults[appInfo.appName];
156+
}
151157
}
152158

153159
// function update(opts: Options, appInfo: PropsInfo) {
@@ -163,7 +169,7 @@ function unmount(opts: Options, appInfo: PropsInfo) {
163169
// });
164170
// }
165171

166-
function atLeastReact18(React: typeReact) {
172+
function atLeastReact18(React?: typeReact) {
167173
if (
168174
React &&
169175
typeof React.version === 'string' &&
@@ -198,15 +204,15 @@ function callCreateRoot({ opts, elementToRender, domElement }) {
198204
}
199205

200206
function getElementToRender(opts: Options, appInfo: PropsInfo) {
201-
const rootComponentElement = opts.React.createElement(
207+
const rootComponentElement = opts.React?.createElement(
202208
opts.rootComponent as any,
203209
appInfo,
204210
);
205211

206212
let elementToRender = rootComponentElement;
207213

208214
if (opts.errorBoundary) {
209-
elementToRender = opts.React.createElement(
215+
elementToRender = opts.React?.createElement(
210216
createErrorBoundary(opts) as any,
211217
appInfo,
212218
elementToRender,
@@ -220,7 +226,7 @@ function createErrorBoundary(opts: Options) {
220226
// to avoid bloat
221227
function GarfishSubAppReactErrorBoundary(this: any, appInfo: PropsInfo) {
222228
// super
223-
opts.React.Component.apply(this, arguments);
229+
opts.React?.Component.apply(this, arguments);
224230

225231
this.state = {
226232
caughtError: null,
@@ -232,15 +238,14 @@ function createErrorBoundary(opts: Options) {
232238
).displayName = `ReactBridgeReactErrorBoundary(${appInfo.appName})`;
233239
}
234240

235-
GarfishSubAppReactErrorBoundary.prototype = Object.create(
236-
opts.React.Component.prototype,
237-
);
241+
GarfishSubAppReactErrorBoundary.prototype =
242+
opts.React && Object.create(opts.React.Component.prototype);
238243

239244
GarfishSubAppReactErrorBoundary.prototype.render = function () {
240245
if (this.state.caughtError) {
241246
const errorBoundary = opts.errorBoundary;
242247

243-
return errorBoundary(this.state.caughtError, this.props);
248+
return errorBoundary && errorBoundary(this.state.caughtError, this.props);
244249
} else {
245250
return this.props.children;
246251
}

packages/bridge-react-v18/tsconfig.json

Lines changed: 0 additions & 6 deletions
This file was deleted.

packages/bridge-vue-v2/src/types.d.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,7 @@ export type OptionalType<T extends new (...args: any) => any> = {
2727
handleInstance: (vueInstance: InstanceType<T>, opts: PropsInfo) => void;
2828
};
2929

30-
export type UserOptions<T, U> = TypeComponent<U> & Partial<OptionalType<T>>;
30+
export type UserOptions<
31+
T extends new (...args: any) => any,
32+
U,
33+
> = TypeComponent<U> & Partial<OptionalType<T>>;

packages/browser-snapshot/__tests__/event.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,12 @@ describe('test sandbox ', () => {
44
it('dom sandbox', () => {
55
const event = new Event('flag-event');
66
const evn = new PatchEvent();
7-
let flag1 = null;
7+
let flag1: string | null = null;
88
window.addEventListener('flag-event', () => (flag1 = 'flag1'));
99

1010
evn.activate();
1111

12-
let flag2 = null;
12+
let flag2: string | null = null;
1313
window.addEventListener('flag-event', () => (flag2 = 'flag2'));
1414

1515
evn.deactivate();

packages/browser-snapshot/__tests__/sandbox.spec.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,11 @@ describe('test sandbox ', () => {
66
<style>body{color: 'yellow';}</style>
77
`;
88

9-
const obj = {
9+
const obj: {
10+
name: string;
11+
age: string;
12+
some?: string;
13+
} = {
1014
name: 'zhoushaw',
1115
age: '24',
1216
some: 'test delete',

packages/browser-snapshot/__tests__/style.spec.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { PatchStyle } from '../src/patchers/style';
2+
import assert from 'assert';
23

34
const createStyleComponentElementWithRecord = () => {
45
const styleComponentElement = document.createElement('style');
@@ -45,6 +46,7 @@ describe('test sandbox ', () => {
4546
sty.activate();
4647
const styleComponentElement = createStyleComponentElementWithRecord();
4748

49+
assert(styleComponentElement.sheet);
4850
expectIncludeCssRules(styleComponentElement.sheet.cssRules);
4951

5052
sty.deactivate();

packages/browser-snapshot/src/patchers/interceptor.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ export class Interceptor {
8282
for (let i = 0; i < cssRules.length; i++) {
8383
const cssRule = cssRules[i];
8484
// re-insert rules for styled-components element
85-
val.sheet.insertRule(cssRule.cssText, val.sheet.cssRules.length);
85+
val.sheet?.insertRule(cssRule.cssText, val.sheet?.cssRules.length);
8686
}
8787
}
8888
}
@@ -106,7 +106,11 @@ export class Interceptor {
106106
created = createdOrSnapshot;
107107
}
108108
created.arrDoms.reduce((prev, val) => {
109-
if (val instanceof HTMLStyleElement && isStyledComponentsLike(val)) {
109+
if (
110+
val instanceof HTMLStyleElement &&
111+
isStyledComponentsLike(val) &&
112+
val?.sheet?.cssRules
113+
) {
110114
this.styledComponentCSSRulesMap.set(val, val.sheet.cssRules);
111115
}
112116
prev.removeChild(val);

0 commit comments

Comments
 (0)