refactor: typescript implement #148

dancerphil · 2021-10-26T10:30:58Z

Working in Progress

Do not merge.

This will let us provide d.ts in next major or minor release. #135

Something will include in this PR

ts implement for site

Something will not include in this but in next PR

remove recompose
use @testing-libary/react-hooks to add some tests

otakustay · 2021-10-27T01:59:59Z

scripts/build.sh

-node rollup.js
-node node_modules/.bin/postcss -o style/index.css src/styles/index.css
-node scripts/fix-css.js style/index.css
+rm -rf cjs/ && tsc


恐怕我们不能现在就改成tsc编译。之前版本是一个bundle，它可以直接用于<script>引入等方式，而tsc的产物并不是打包的，这会是一个BREAKING

你觉得应该怎么处理？

在这个大版本中，乖乖保留原来的rollup构建吧，下个大版本改为tsc

rollup 怎么生成 d.ts 啊

版本控制可以交给你，我这个能发个 3.0-alpha.0 就好？
我想你后续对 3.0 也有很多构想吧。你可以在这个基础上搞。

让tsc生成定义，然后用rollup生成bundle，这样行不？入口是index.js -> index.d.ts，能对上应该就没问题

可能的问题

dependencies 要不要加上，因为可能会依赖他们的类型

import {xxx} from 'react-diff-view/es/tokenize' 会被 ts 支持，但是 js 不支持，因为类型好像必须是 esm 的，没法合起来，不过也许可以只覆盖一个 index.js，其他的还是 esm？不知道 script 引入的要求是什么？

otakustay · 2021-10-27T02:04:39Z

src/Decoration/UnifiedDecoration.tsx

@@ -19,6 +26,7 @@ const UnifiedDecoration = ({hideGutter, className, gutterClassName, contentClass
        );
    }

+    // @ts-ignore


这是children的类型问题？这个类型能不能再强化一下，弄成tuple的话，jsx能不能认出来……

应该只是 ts 类型推断上的问题，Children.count() 不会有推导，实际是对的，所以 ignore 了，不影响使用

otakustay · 2021-10-27T02:05:45Z

src/Diff/index.tsx

+    viewType,
+    gutterType = 'default',
+    selectedChanges = [],
+    widgets = {},


这里个[]和{}，最好放外面定义个常量？不然用默认值的时候，每次都是新对象，而react-diff-view整个性能非常依赖memo

otakustay · 2021-10-27T02:11:35Z

src/context/index.ts

+import {createContext, useContext} from 'react';
+
+interface Context {
+    gutterType: 'default' | 'none' | 'anchor';


这几个string literal union感觉应该是单独的类型，用到的地方不少

otakustay · 2021-10-27T02:12:05Z

src/context/index.ts

+    viewType: 'unified' | 'split';
+    selectedChanges: string[];
+    monotonous: boolean;
+    widgets: {


用Record<string, ReactNode>是不是比较好

otakustay · 2021-10-27T02:12:21Z

src/context/index.ts

+    widgets: {
+        [key: string]: any;
+    };
+    renderGutter: () => any;


返回应该是ReactNode

otakustay · 2021-10-27T02:17:15Z

src/interface.ts

+    type: 'insert';
+    content: string;
+    isInsert: true;
+    isDelete?: boolean;


这里应该固定值，没有或者false吧，都知道是insert了

这里严格会导致需要改代码，就不是初衷了

otakustay · 2021-10-27T02:18:21Z

src/interface.ts

+
+export type Change = InsertChange | DeleteChange | NormalChange;
+
+export interface HunkType {


这个类型的命名我有点想法，一般XxxType会给人是一种枚举的感觉，叫HunkInfo之类的会不会更好，下同

otakustay · 2021-10-27T02:18:38Z

src/interface.ts

+    isBinary: boolean;
+    // 'rename' 和 'copy' 理论上存在但实际很少用到
+    type: 'add' | 'delete' | 'modify' | 'rename' | 'copy';
+


多了空行

otakustay · 2021-10-27T02:20:12Z

src/interface.ts

+    generateAnchorID?: (change: Change) => any;
+    selectedChanges?: string[];
+    tokens?: any[] | null;
+    widgets?: {


统一用Record吧，我记得我们的TS lint规则里有一条这个，只是react-diff-view现在没用我们的规则

好像没有，我从内部挪过来的

otakustay · 2021-10-27T02:20:55Z

src/utils/diff/expandCollapsedBlockBy.ts

@@ -57,7 +64,7 @@ const findNearestNormalChangeIndex = ({changes}, start) => {
    return -1;
 };

-const splitRangeToValidOnes = (hunks, start, end) => {
+const splitRangeToValidOnes = (hunks: HunkType[], start: number, end: number): Array<[number, number]> => {


把[number, number]拉出来造一个Range的类型更有语义

这个 PR 不会改语义，合入之后可以改。

我的意思是，放一个type Range = [number number]，在类型上更有可读性

哦，明白了

stale · 2021-11-10T14:58:44Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

nghiepdev · 2021-12-17T03:55:38Z

Up!!!

garyray-k · 2023-01-25T03:20:52Z

👀

otakustay · 2023-02-20T04:34:32Z

I decide to move all implementation to TypeScript and publish version 2 with some small breaking changes, please track #189 for progress.

otakustay reviewed Oct 27, 2021

View reviewed changes

otakustay requested changes Oct 27, 2021

View reviewed changes

dancerphil force-pushed the master branch from 7e707f8 to 07e1acd Compare October 27, 2021 06:44

refactor: typescript implement

e89b529

dancerphil force-pushed the master branch from 07e1acd to e89b529 Compare October 27, 2021 06:58

stale bot added the stale label Nov 10, 2021

otakustay added the pinned label Nov 10, 2021

stale bot removed the stale label Nov 10, 2021

This was referenced Jul 1, 2022

[react-diff-view] Add types for react-diff-view DefinitelyTyped/DefinitelyTyped#60921

Closed

Types react diff view oBusk/npm-diff.app#615

Closed

otakustay closed this Feb 20, 2023


		export type Change = InsertChange \| DeleteChange \| NormalChange;

		export interface HunkType {

refactor: typescript implement #148

refactor: typescript implement #148

Uh oh!

Conversation

dancerphil commented Oct 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Working in Progress

Something will include in this PR

Something will not include in this but in next PR

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dancerphil Oct 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stale bot commented Nov 10, 2021

Uh oh!

nghiepdev commented Dec 17, 2021

Uh oh!

garyray-k commented Jan 25, 2023

Uh oh!

otakustay commented Feb 20, 2023

Uh oh!

dancerphil commented Oct 26, 2021 •

edited

Loading

dancerphil Oct 27, 2021 •

edited

Loading