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

Update frontend libraries and adopt React compiler #209

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ node_modules
webdiff/static/components
webdiff/static/js/file_diff.js
webdiff/static/js/file_diff.js.map
webdiff/static/js/file_diff.js.LICENSE.txt

wheelhouse
.vscode
Expand Down
38 changes: 16 additions & 22 deletions ts/CodeDiffContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -164,29 +164,23 @@ function FileDiff(props: FileDiffProps) {
// First guess a language based on the file name.
// Fall back to guessing based on the contents of the longer version.
const path = pathBefore || pathAfter;
const language = React.useMemo(() => {
let language = guessLanguageUsingFileName(path);
if (
!language &&
!HIGHLIGHT_BLACKLIST.includes(extractFilename(path)) &&
numLines < GIT_CONFIG.webdiff.maxLinesForSyntax
) {
let byLength = [contentsBefore, contentsAfter];
if (contentsAfter && lengthOrZero(contentsAfter) > lengthOrZero(contentsBefore)) {
byLength = [byLength[1], byLength[0]];
}
language = byLength[0] ? guessLanguageUsingContents(byLength[0]) ?? null : null;
let language = guessLanguageUsingFileName(path);
if (
!language &&
!HIGHLIGHT_BLACKLIST.includes(extractFilename(path)) &&
numLines < GIT_CONFIG.webdiff.maxLinesForSyntax
) {
let byLength = [contentsBefore, contentsAfter];
if (contentsAfter && lengthOrZero(contentsAfter) > lengthOrZero(contentsBefore)) {
byLength = [byLength[1], byLength[0]];
}
return language;
}, [contentsAfter, contentsBefore, numLines, path]);

const opts = React.useMemo(
(): Partial<PatchOptions> => ({
language,
// TODO: thread through minJumpSize
}),
[language],
);
language = byLength[0] ? guessLanguageUsingContents(byLength[0]) ?? null : null;
}

const opts: Partial<PatchOptions> = {
language,
// TODO: thread through minJumpSize
};

return (
<div className="diff">
Expand Down
4 changes: 1 addition & 3 deletions ts/FileList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@ export function FileList(props: Props) {
const {filePairs, selectedIndex, fileChangeHandler} = props;

const anyWithDiffstats = filePairs.some(fp => fp.num_add !== null || fp.num_delete !== null);
const maxDelta = React.useMemo(() => {
return Math.max(1, ...filePairs.map(fp => (fp.num_add ?? 0) + (fp.num_delete ?? 0)));
}, [filePairs]);
const maxDelta = Math.max(1, ...filePairs.map(fp => (fp.num_add ?? 0) + (fp.num_delete ?? 0)));

const lis = filePairs.map((filePair, idx) => {
const displayName = filePairDisplayName(filePair);
Expand Down
21 changes: 9 additions & 12 deletions ts/ImageBlinker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,15 @@ export function ImageBlinker(props: ImageDiffProps) {
}
};

const blink = React.useCallback(
(e: KeyboardEvent) => {
if (!isLegitKeypress(e)) {
return;
}
if (e.key === 'b') {
setAutoBlink(false);
setIdx(idx => 1 - idx);
}
},
[setIdx, setAutoBlink],
);
const blink = React.useCallback((e: KeyboardEvent) => {
if (!isLegitKeypress(e)) {
return;
}
if (e.key === 'b') {
setAutoBlink(false);
setIdx(idx => 1 - idx);
}
}, []);

// XXX old version also sets this on a[value="blink"], what is that?
React.useEffect(() => {
Expand Down
26 changes: 13 additions & 13 deletions ts/Root.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';
import {RouteComponentProps, useHistory} from 'react-router';
import {useNavigate, useParams} from 'react-router-dom';
import {FilePair} from './CodeDiffContainer';
import {DiffOptions} from './diff-options';
import {DiffView, PerceptualDiffMode} from './DiffView';
Expand All @@ -15,10 +15,8 @@ declare const pairs: FilePair[];
declare const initialIdx: number;
declare const GIT_CONFIG: GitConfig;

type Props = RouteComponentProps<{index?: string}>;

// Webdiff application root.
export function Root(props: Props) {
export function Root() {
const [pdiffMode, setPDiffMode] = React.useState<PerceptualDiffMode>('off');
const [imageDiffMode, setImageDiffMode] = React.useState<ImageDiffMode>('side-by-side');
const [diffOptions, setDiffOptions] = React.useState<Partial<DiffOptions>>({});
Expand All @@ -30,22 +28,24 @@ export function Root(props: Props) {
pairs.length <= 6 ? 'list' : 'dropdown',
);

const history = useHistory();
const selectIndex = React.useCallback(
(idx: number) => {
history.push(`/${idx}`);
},
[history],
);
const navigate = useNavigate();
const selectIndex = (idx: number) => {
navigate(`/${idx}`);
};

const idx = Number(props.match.params.index ?? initialIdx);
const params = useParams<'index'>();
const idx = Number(params.index ?? initialIdx);
const filePair = pairs[idx];
// TODO: use react-helmet
React.useEffect(() => {
document.title = 'Diff: ' + filePairDisplayName(filePair) + ' (' + filePair.type + ')';
}, [filePair]);

// TODO: switch to useKey() or some such
React.useEffect(() => {
const selectIndex = (idx: number) => {
navigate(`/${idx}`);
};
const handleKeydown = (e: KeyboardEvent) => {
if (!isLegitKeypress(e)) return;
if (e.code == 'KeyK') {
Expand All @@ -70,7 +70,7 @@ export function Root(props: Props) {
return () => {
document.removeEventListener('keydown', handleKeydown);
};
}, [idx, selectIndex]);
}, [idx, navigate]);

const inlineStyle = `
td.code {
Expand Down
26 changes: 10 additions & 16 deletions ts/codediff/codediff.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const DEFAULT_PARAMS: PatchOptions = {
* tags will be balanced within each line.
*/
function highlightText(text: string, language: string): string[] | null {
console.log('highlightText');
// TODO(danvk): look into suppressing highlighting if .relevance is low.
const html = hljs.highlight(text, {language, ignoreIllegals: true}).value;

Expand Down Expand Up @@ -60,22 +61,14 @@ export interface Props {
export function CodeDiff(props: Props) {
const {beforeText, afterText, ops, params} = props;

const beforeLines = React.useMemo(
() => (beforeText ? stringAsLines(beforeText) : []),
[beforeText],
);
const afterLines = React.useMemo(() => (afterText ? stringAsLines(afterText) : []), [afterText]);
const fullParams = React.useMemo(() => ({...DEFAULT_PARAMS, ...params}), [params]);
const diffRanges = React.useMemo(
() => enforceMinJumpSize(ops, fullParams.minJumpSize),
[ops, fullParams],
);
const beforeLines = beforeText ? stringAsLines(beforeText) : [];
const afterLines = afterText ? stringAsLines(afterText) : [];
const fullParams = {...DEFAULT_PARAMS, ...params};
const diffRanges = enforceMinJumpSize(ops, fullParams.minJumpSize);
const {language} = fullParams;

const [beforeLinesHighlighted, afterLinesHighlighted] = React.useMemo(() => {
if (!language) return [null, null];
return [highlightText(beforeText ?? '', language), highlightText(afterText ?? '', language)];
}, [language, beforeText, afterText]);
const beforeLinesHighlighted = language ? highlightText(beforeText ?? '', language) : null;
const afterLinesHighlighted = language ? highlightText(afterText ?? '', language) : null;

return (
<CodeDiffView
Expand Down Expand Up @@ -132,7 +125,8 @@ interface CodeDiffViewProps {
ops: readonly DiffRange[];
}

const CodeDiffView = React.memo((props: CodeDiffViewProps) => {
function CodeDiffView(props: CodeDiffViewProps) {
console.log('render CodeDiffView');
const {
filePair,
params,
Expand Down Expand Up @@ -284,7 +278,7 @@ const CodeDiffView = React.memo((props: CodeDiffViewProps) => {
</table>
</div>
);
});
}

function HeaderRow({filePair}: {filePair: FilePair}) {
const {a, b, num_add, num_delete} = filePair;
Expand Down
21 changes: 15 additions & 6 deletions ts/eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,14 @@

import eslint from '@eslint/js';
import tseslint from 'typescript-eslint';
import reactCompilerPlugin from 'eslint-plugin-react-compiler';
import hooksPlugin from 'eslint-plugin-react-hooks';

export default tseslint.config(
eslint.configs.recommended,
// See https://typescript-eslint.io/users/configs
...tseslint.configs.strictTypeChecked,
...tseslint.configs.stylisticTypeChecked,
{
plugins: {
'react-hooks': hooksPlugin,
},
rules: hooksPlugin.configs.recommended.rules,
},
{
languageOptions: {
parserOptions: {
Expand All @@ -25,6 +20,12 @@ export default tseslint.config(
},
},
},
{
plugins: {
'react-hooks': hooksPlugin,
},
rules: hooksPlugin.configs.recommended.rules,
},
{
rules: {
'@typescript-eslint/restrict-template-expressions': [
Expand All @@ -48,4 +49,12 @@ export default tseslint.config(
// '@typescript-eslint/no-non-null-assertion': 'off',
},
},
{
plugins: {
'react-compiler': reactCompilerPlugin,
},
rules: {
'react-compiler/react-compiler': 'error',
},
},
);
23 changes: 15 additions & 8 deletions ts/index.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,23 @@
import {createRoot} from 'react-dom/client';
import React from 'react';
import ReactDOM from 'react-dom';
import {BrowserRouter as Router, Switch, Route} from 'react-router-dom';
import {BrowserRouter, Routes, Route} from 'react-router-dom';
import {injectStylesFromConfig} from './options';
import {Root} from './Root';

const App = () => (
<Router>
<Switch>
<Route path="/:index?" component={Root} />
</Switch>
</Router>
<BrowserRouter>
<Routes>
<Route path="/:index?" element={<Root />} />
</Routes>
</BrowserRouter>
);

injectStylesFromConfig();
ReactDOM.render(<App />, document.getElementById('application'));
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const root = createRoot(document.getElementById('application')!);

root.render(
<React.StrictMode>
<App />
</React.StrictMode>,
);
25 changes: 15 additions & 10 deletions ts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"author": "Dan Vanderkam ([email protected])",
"license": "MIT",
"scripts": {
"build": "NODE_OPTIONS=--openssl-legacy-provider webpack",
"build": "webpack",
"format": "yarn prettier --write '**/*.ts{,x}'",
"format:check": "yarn prettier --check '**/*.ts{,x}'",
"watch": "yarn run build --watch",
Expand All @@ -21,30 +21,35 @@
"@types/jest": "^29.5.12",
"@types/lodash": "^4.14.161",
"@types/node": "^20.14.2",
"@types/react": "^16.9.49",
"@types/react-dom": "^16.9.8",
"@types/react-router": "^5.1.8",
"@types/react": "npm:types-react@rc",
"@types/react-dom": "npm:types-react-dom@rc",
"@types/react-router-dom": "^5.1.5",
"babel-plugin-react-compiler": "^0.0.0-experimental-938cd9a-20240601",
"eslint": "^9.4.0",
"eslint-plugin-react-compiler": "^0.0.0-experimental-51a85ea-20240601",
"eslint-plugin-react-hooks": "^5.1.0-rc-cc1ec60d0d-20240607",
"jest": "^29.7.0",
"jest-environment-jsdom": "^29.7.0",
"knip": "^5.17.4",
"prettier": "^2.1.2",
"react-compiler-webpack": "^0.1.2",
"ts-jest": "^29.1.4",
"ts-loader": "^8.0.4",
"typescript": "^5.4.5",
"typescript-eslint": "^7.12.0",
"webpack": "^4.44.2",
"webpack-cli": "^3.3.12"
"webpack": "^5.92.0",
"webpack-cli": "^5.1.4"
},
"dependencies": {
"diff": "^5.2.0",
"highlight.js": "^11.9.0",
"lodash": "^4.17.20",
"react": "^16.13.1",
"react-dom": "^16.13.1",
"react-router": "^5.2.0",
"react-router-dom": "^5.2.0"
"react": "^19.0.0-rc-f3e09d6328-20240612",
"react-dom": "^19.0.0-rc-f3e09d6328-20240612",
"react-router-dom": "^6.23.1"
},
"overrides": {
"@types/react": "npm:types-react@rc",
"@types/react-dom": "npm:types-react-dom@rc"
}
}
20 changes: 19 additions & 1 deletion ts/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,30 @@ const path = require('path');

const mode = process.env.NODE_ENV || 'production';

const {defineReactCompilerLoaderOption, reactCompilerLoader} = require('react-compiler-webpack');

const options = {
mode: mode,
entry: './index.tsx',
devtool: 'sourcemap',
devtool: 'source-map',
module: {
rules: [
{
test: /\.[mc]?[jt]sx$/i,
exclude: /node_modules/,
use: [
// babel-loader, swc-loader, esbuild-loader, or anything you like to transpile JSX should go here.
// If you are using rspack, the rspack's buiilt-in react transformation is sufficient.
// { loader: 'swc-loader' },
// Now add forgetti-loader
{
loader: reactCompilerLoader,
options: defineReactCompilerLoaderOption({
// React Compiler options goes here
}),
},
],
},
{
test: /\.tsx?$/,
use: 'ts-loader',
Expand Down
Loading