Skip to content

Use native node for running the bot#562

Open
JstnMcBrd wants to merge 10 commits into
mainfrom
mcb/native-node-ts
Open

Use native node for running the bot#562
JstnMcBrd wants to merge 10 commits into
mainfrom
mcb/native-node-ts

Conversation

@JstnMcBrd
Copy link
Copy Markdown
Contributor

Follow up to #541

Modernizes and simplifies our setup

Changes

  • Fixed the required node version
    • Type-stripping is not unflagged in all version of Node 23, so it should be excluded
  • Rewrote all imports from .js to .ts
  • Enforced erasableSyntaxOnly
    • This means not using enums - only downside
  • Pointed node directly to src/main.ts
  • Removed the build step
  • Replaced it with a type-checking script
  • Removed all references to dist
  • Updated the Dockerfile to reflect build changes
  • Excluded test files from type-checking and typed linting
    • There's always been a ton of unaddressed type errors in our test files, but the build step never caught them because it excluded test files
    • The type-checking step doesn't exclude test files, so all the type errors are visible now
    • There's way too many to fix right now, so I'm saving it for later

@JstnMcBrd JstnMcBrd requested a review from a team May 13, 2026 11:47
@JstnMcBrd JstnMcBrd self-assigned this May 13, 2026
@JstnMcBrd JstnMcBrd added the enhancement New feature or request label May 13, 2026
@JstnMcBrd
Copy link
Copy Markdown
Contributor Author

The branch protection rules need to be updated to expect CI / Type check instead of CI / Build.

Comment thread package.json
"dectalk-tts": "1.0.1",
"discord.js": "14.26.4",
"ffmpeg-static": "5.3.0",
"source-map-support": "0.5.21",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we go this route, we can probably do away with source-map-support too, since we no longer have a "build" stage to generate those.

Comment thread Dockerfile

ENV NODE_ENV=production

RUN apt-get update -y
Copy link
Copy Markdown
Contributor

@AverageHelper AverageHelper May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about removing this line. Shouldn't we update the package lists before trying to install packages?

@AverageHelper
Copy link
Copy Markdown
Contributor

I'm not sure on removing the "build" stage yet. I'm hoping to eventually set up something like Rollup so we can tree-shake and avoid including node_modules in the final bundle, and setting that up would require reinstating that build step and possibly undoing this entire PR later 😓

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants