refactor: major project structure update #1564

setchy · 2024-09-30T21:02:39Z

Major project structure update applying

learnings from building atlassify
converge closer to the latest https://electron-react-boilerplate.js.org/ example

Main updates include:

split code into main and renderer src components for clear separation of duties
update main to use TypeScript and split into smaller files (main, menu, icons, utils, updater)
improve webpack configs, including code minimizing for production builds
use webpack plugins for templating html and extracting css
use webpack plugins to copy twemoji svgs into final build with updated EmojiText implementation to avoid reading directly from node_modules path.
move all non-core dependencies from dependencies to devDependencies
rename jest test suites to match new folder structure

…n, and improve webpack process for bundling Signed-off-by: Adam Setch <[email protected]>

setchy · 2024-09-30T21:07:38Z

src/renderer/components/EmojiText.tsx

@@ -21,13 +21,12 @@ export const EmojiText: FC<IEmojiText> = ({ text }) => {
      ref.current.innerHTML = twemoji.parse(text, {
        folder: 'svg',
        ext: '.svg',
-        callback: (icon: string, options: TwemojiOptions, _variant: string) => {


No longer need to pluck these out from the node_modules within the asar bundle 🚀

…n, and improve webpack process for bundling Signed-off-by: Adam Setch <[email protected]>

Signed-off-by: Adam Setch <[email protected]>

…n, and improve webpack process for bundling Signed-off-by: Adam Setch <[email protected]>

Signed-off-by: Adam Setch <[email protected]>

bmulholland · 2024-10-01T09:18:09Z

Any way to split this into chunks, to ease review? :)

setchy · 2024-10-01T12:56:14Z

Any way to split this into chunks, to ease review? :)

Yep - can be done...

afonsojramos · 2024-10-01T14:54:44Z

.github/workflows/release.yml

        env:
          GH_TOKEN: ${{ secrets.github_token }}
+      - run: pnpm prepare:remove-source-maps


Why do we need to remove the source-maps?

Why is this one after the package and on others before the package?

Good eyes re: 2 - that's an oversight on my part. Updated

re 1 - it was within the https://github.com/electron-react-boilerplate/electron-react-boilerplate example. My assumption that they remove the source maps so that the packaged+minified code cannot be reconstructed. Not a big deal for an open source project

Exactly, and it may help with tracing back bugs reported by users, however I am not sure how we can help users provide these off the top of my head 🤔

Yeah, not too sure. I think our exception handling and electron-logger should be catching most user reported issues.

That said, would it be best for now to skip minimizing source code?

Signed-off-by: Adam Setch <[email protected]>

afonsojramos

Looking good! A few comments, but I think this may make it easier for us to switch from Webpack to Vite in the future 👀

afonsojramos · 2024-10-01T14:58:17Z

package.json

-    "@discordapp/twemoji": "15.1.0",
    "@electron/remote": "2.1.2",
-    "@primer/octicons-react": "19.11.0",
-    "axios": "1.7.7",
-    "class-variance-authority": "0.7.0",
-    "clsx": "2.1.1",
-    "date-fns": "4.1.0",
    "electron-log": "5.2.0",
    "electron-updater": "6.3.4",
-    "final-form": "4.20.10",
    "menubar": "9.5.0",
-    "nprogress": "0.2.0",
    "react": "18.3.1",
    "react-dom": "18.3.1",
-    "react-final-form": "6.5.9",
    "react-router-dom": "6.26.2",
-    "tailwind-merge": "2.5.2",
-    "ts-loader": "9.5.1",
-    "typescript": "5.6.2",
    "update-electron-app": "3.0.0"


Feels weird that a lot of these can go in the devDependencies. Are you sure about nprogress, class-variance-authority, axios, @primer/octicons-react, @discordapp/twemoji, clsx, tailwind-merge and react-final-form?

Seems to be fine - in fact more probably can be moved actually.

We can validate in the release candidate testing

What's even the difference between the two, in this case? I feel like it makes sense for e.g. a server, where you have execution mode and development mode. But for Gitify, the code that's run in execution mode is all post-build, and the dev dependencies are used to make the build. So the package.json is never actually executed in a non-dev mode. If that makes sense?

afonsojramos · 2024-10-01T15:01:22Z

src/renderer/utils/cn.ts

@@ -1,4 +1,4 @@
-import { type ClassValue, clsx } from 'clsx/lite';
+import { type ClassValue, clsx } from 'clsx';


Did you find a reason for needing the non lite one? I had set that one because we didn't need the base one.

No specific use-case, but was having issues with the new tsconfig and importing clsx/lite.

afonsojramos · 2024-10-01T15:02:11Z

config/webpack.config.main.prod.ts

+import baseConfig from './webpack.config.main.base';
+
+const configuration: webpack.Configuration = {
+  devtool: 'source-map',


Do we need this in the prod one? - I suppose this relates to us deleting them on the build step. We might keep it if we want to, depends on what gets decided in #1564 (comment)

🤷‍♂️ - was just following the erb template

IMO source-maps make sense if we have a centralized error reporting. That's probably why the template has it. Since we don't have exception tracking, there's nowhere to send the source maps, so I don't think it's necessary.

src/renderer/index.html

src/renderer/index.tsx

Signed-off-by: Adam Setch <[email protected]>

src/renderer/App.css

refactor: split code into main and renderer modules, adopt TS for mai…

d16c501

…n, and improve webpack process for bundling Signed-off-by: Adam Setch <[email protected]>

setchy requested review from afonsojramos and bmulholland as code owners September 30, 2024 21:02

github-actions bot added dependency Dependency updates refactor Refactoring of existing feature labels Sep 30, 2024

setchy commented Sep 30, 2024

View reviewed changes

setchy added 5 commits September 30, 2024 17:32

refactor: split code into main and renderer modules, adopt TS for mai…

5eeed57

…n, and improve webpack process for bundling Signed-off-by: Adam Setch <[email protected]>

refactor: split code into main and renderer modules, adopt TS for mai…

56e46d3

…n, and improve webpack process for bundling Signed-off-by: Adam Setch <[email protected]>

refactor: split code into main and renderer modules, adopt TS for mai…

877635e

…n, and improve webpack process for bundling Signed-off-by: Adam Setch <[email protected]>

refactor: split code into main and renderer modules, adopt TS for mai…

4fb5405

…n, and improve webpack process for bundling Signed-off-by: Adam Setch <[email protected]>

refactor: split code into main and renderer modules, adopt TS for mai…

964db86

…n, and improve webpack process for bundling Signed-off-by: Adam Setch <[email protected]>

setchy marked this pull request as draft September 30, 2024 22:16

refactor: split code into main and renderer modules, adopt TS for mai…

26448b2

…n, and improve webpack process for bundling Signed-off-by: Adam Setch <[email protected]>

setchy marked this pull request as ready for review September 30, 2024 22:37

setchy added 5 commits September 30, 2024 19:03

build: universal only

a067147

Signed-off-by: Adam Setch <[email protected]>

refactor: split code into main and renderer modules, adopt TS for mai…

3959bd7

…n, and improve webpack process for bundling Signed-off-by: Adam Setch <[email protected]>

build: remove resize polyfill dep

66ef544

Signed-off-by: Adam Setch <[email protected]>

build: macos dist

01a1eed

Signed-off-by: Adam Setch <[email protected]>

build: macos dist

67d56b5

Signed-off-by: Adam Setch <[email protected]>

afonsojramos reviewed Oct 1, 2024

View reviewed changes

build: fix order of source maps

39771f0

Signed-off-by: Adam Setch <[email protected]>

afonsojramos requested changes Oct 1, 2024

View reviewed changes

setchy requested a review from afonsojramos October 1, 2024 15:25

bmulholland reviewed Oct 2, 2024

View reviewed changes

src/renderer/index.html Outdated Show resolved Hide resolved

bmulholland reviewed Oct 2, 2024

View reviewed changes

src/renderer/index.tsx Show resolved Hide resolved

setchy requested a review from bmulholland October 2, 2024 16:28

Merge branch 'main' into refactor/major-overhaul

ea50d6a

Signed-off-by: Adam Setch <[email protected]>

bmulholland reviewed Oct 2, 2024

View reviewed changes

src/renderer/App.css Show resolved Hide resolved

bmulholland approved these changes Oct 2, 2024

View reviewed changes

setchy merged commit 992be09 into main Oct 4, 2024
10 checks passed

setchy deleted the refactor/major-overhaul branch October 4, 2024 19:08

setchy added this to the Release 5.15.0 milestone Oct 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: major project structure update #1564

refactor: major project structure update #1564

setchy commented Sep 30, 2024 •

edited

Loading

setchy Sep 30, 2024

bmulholland commented Oct 1, 2024

setchy commented Oct 1, 2024

afonsojramos Oct 1, 2024

setchy Oct 1, 2024

setchy Oct 1, 2024

afonsojramos Oct 1, 2024

setchy Oct 1, 2024

afonsojramos left a comment

afonsojramos Oct 1, 2024

setchy Oct 1, 2024

setchy Oct 1, 2024

bmulholland Oct 2, 2024

afonsojramos Oct 1, 2024

setchy Oct 1, 2024 •

edited

Loading

afonsojramos Oct 1, 2024 •

edited

Loading

setchy Oct 1, 2024 •

edited

Loading

bmulholland Oct 2, 2024

		@@ -1,4 +1,4 @@
		import { type ClassValue, clsx } from 'clsx/lite';
		import { type ClassValue, clsx } from 'clsx';

refactor: major project structure update #1564

refactor: major project structure update #1564

Conversation

setchy commented Sep 30, 2024 • edited Loading

Choose a reason for hiding this comment

bmulholland commented Oct 1, 2024

setchy commented Oct 1, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afonsojramos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

setchy Oct 1, 2024 • edited Loading

Choose a reason for hiding this comment

afonsojramos Oct 1, 2024 • edited Loading

Choose a reason for hiding this comment

setchy Oct 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

setchy commented Sep 30, 2024 •

edited

Loading

setchy Oct 1, 2024 •

edited

Loading

afonsojramos Oct 1, 2024 •

edited

Loading

setchy Oct 1, 2024 •

edited

Loading