X Tutup
The Wayback Machine - https://web.archive.org/web/20200915024125/https://github.com/cs01/gdbgui/pull/256
Skip to content

/ gdbgui

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

Add TypeScript, catch bugs #256

Merged
merged 4 commits into from Apr 28, 2019
Merged

Add TypeScript, catch bugs #256

merged 4 commits into from Apr 28, 2019

Conversation

@bcherny
Copy link
Contributor

bcherny commented Jan 9, 2019 •
edited

This PR:

  • Adds TypeScript, TSLint, and TSJest.
  • Switches the JS build pipeline from Babel to TSC (alternatively, we can use both, but since the UI is so simple, it's simpler to just use TSC without Babel). TSC now compiles both .ts and .js/.jsx files.
  • Ports Links.jsx to TypeScript, catching a few bugs along the way.

Tests pass, but I'd love help testing this end to end. It's likely that this doesn't work at runtime yet, and will require a little more tweaking.
@bcherny
bcherny reviewed Jan 9, 2019
View changes
gdbgui/src/js/Links.tsx
render() {
let line = parseInt(this.props.line);
let onclick = null,
let onclick = () => {},

This comment has been minimized.

Sign in to view
@bcherny

bcherny Jan 9, 2019

Author Contributor

Bug #2: React event handlers shouldn't be null.
gdbgui/src/js/Links.tsx
cls = "";
if (!this.props.file || !line) {
line = "";
line = 0;

This comment has been minimized.

Sign in to view
@bcherny

bcherny Jan 9, 2019

Author Contributor

Bug #3: line is a number, not a string.
gdbgui/src/js/Links.tsx Outdated
@@ -1,14 +1,22 @@
import Actions from "./Actions.js";
import React from "react";
import CopyToClipboard from "./CopyToClipboard.jsx";
import MemoryLink from "./MemoryLink.jsx"

This comment has been minimized.

Sign in to view
@bcherny

bcherny Jan 9, 2019

Author Contributor

Bug #1: It looks like we forgot to import MemoryLink before?
@bcherny bcherny force-pushed the bcherny:ts branch from 83412e2 to fe7e104 Jan 9, 2019
@cs01
Copy link
Owner

cs01 commented Jan 19, 2019

Thank you for this! It's amazing.

A couple things:

  • the build now takes ~25 sec / 3 sec for build/rebuild on my computer (it used to take ~9 sec / < 1 sec). Would bringing babel back in make this faster?
  • I see this in the build now

No valid rules have been specified for JavaScript files
No valid rules have been specified for TypeScript files

Is this expected?
@bcherny
Copy link
Contributor Author

bcherny commented Jan 28, 2019

Hey @cs01! I just came back from vacation.

the build now takes ~25 sec / 3 sec for build/rebuild on my computer (it used to take ~9 sec / < 1 sec). Would bringing babel back in make this faster?

That's definitely longer than expected. Will take a look in the next 1-2 weeks.

I see this in the build now

Looks like I misconfigured something in Webpack. Will take a look at that one too, probably a 1-liner.
bcherny added 4 commits Jan 9, 2019
@bcherny
init typescript, tslint
f0c8779
@bcherny
port Links to typescript
ebf8031
@bcherny
port CopyToClipboard and MemoryLink to ts
10e6be7
@bcherny
move typechecking to a separate thread to reduce incremental build ti…
1721504 
…mes to <1s
@bcherny bcherny force-pushed the bcherny:ts branch from fe7e104 to 1721504 Mar 10, 2019
@bcherny
Copy link
Contributor Author

bcherny commented Mar 10, 2019

@cs01 Updated!
@cs01
Copy link
Owner

cs01 commented Mar 15, 2019

Thanks! Will take a look when I get the time.
@cs01
Copy link
Owner

cs01 commented Mar 22, 2019 •
edited

I tried it out. It builds with no errors, and finds type errors while it's watching with yarn start. Awesome! 🚀

The build time went from 5814ms to ~30 for intial build, but incremental builds are only ~1 second, so I can live with that.

It looks like the source maps were removed, was there a reason for that? I would prefer to keep them if possible.
@cs01
cs01 reviewed Mar 22, 2019
View changes
webpack.config.js Show resolved Hide resolved
@cs01
Copy link
Owner

cs01 commented Mar 22, 2019

Looking around some more, and it seems like the watcher is a little flaky. have you encountered this? I'm not sure if it's something wrong with my setup or not.

If I run yarn start, after ~30 seconds the build finishes and I see this

> yarn start
yarn run v1.15.2
$ NODE_ENV=development webpack --mode development --watch --config webpack.config.js
Starting type checking and linting service...
Using 1 worker with 2048MB memory limit

Webpack is watching the files…

No type errors found
No lint errors found
Version: typescript 3.3.3333, tslint 5.13.1
Time: 12553ms
Hash: 985759485b38bf6b9883
Version: webpack 4.12.1
Time: 28367ms
Built at: 03/21/2019 5:42:34 PM
       Asset      Size  Chunks             Chunk Names
    build.js  1.11 MiB    main  [emitted]  main
build.js.map  1.24 MiB    main  [emitted]  main
[./gdbgui/src/js/Actions.js] 11.2 KiB {main} [built]
[./gdbgui/src/js/FileOps.jsx] 25.4 KiB {main} [built]
[./gdbgui/src/js/FoldersView.jsx] 12.1 KiB {main} [built]
[./gdbgui/src/js/GdbApi.jsx] 15.3 KiB {main} [built]
[./gdbgui/src/js/GdbConsoleContainer.jsx] 6.84 KiB {main} [built]
[./gdbgui/src/js/GdbguiModal.jsx] 2.08 KiB {main} [built]
[./gdbgui/src/js/GlobalEvents.js] 2.18 KiB {main} [built]
[./gdbgui/src/js/HoverVar.jsx] 4.46 KiB {main} [built]
[./gdbgui/src/js/RightSidebar.jsx] 8.27 KiB {main} [built]
[./gdbgui/src/js/Settings.jsx] 5.53 KiB {main} [built]
[./gdbgui/src/js/ToolTip.jsx] 2.64 KiB {main} [built]
[./gdbgui/src/js/ToolTipTourguide.jsx] 5.49 KiB {main} [built]
[./gdbgui/src/js/TopBar.jsx] 16.1 KiB {main} [built]
[./gdbgui/src/js/constants.js] 2.03 KiB {main} [built]
[./gdbgui/src/js/gdbgui.jsx] 6.74 KiB {main} [built]
    + 50 hidden modules

but when I change a file, nothing happens. I saw this earlier, but killed the yarn start process, then restarted and it worked so I thought it was my imagination. But it is happening again now.
@cs01 cs01 mentioned this pull request Mar 24, 2019
Open
@cs01 cs01 merged commit ab403f0 into cs01:master Apr 28, 2019
@cs01
Copy link
Owner

cs01 commented Apr 28, 2019

Thanks again @bcherny!
cs01 added a commit that referenced this pull request Jul 14, 2020
@bcherny @cs01
Add TypeScript, catch bugs (#256)
f44a331
cs01 added a commit that referenced this pull request Jul 14, 2020
@bcherny @cs01
Add TypeScript, catch bugs (#256)
2943d4a
cs01 added a commit that referenced this pull request Jul 18, 2020
@bcherny @cs01
Add TypeScript, catch bugs (#256)
d4c73c8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cs01 cs01

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
@bcherny @cs01
You can’t perform that action at this time.
X Tutup