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

refactor: rewrite to djs v14 #2505

Merged
merged 27 commits into from
Dec 28, 2023
Merged

refactor: rewrite to djs v14 #2505

merged 27 commits into from
Dec 28, 2023

Conversation

kyranet
Copy link
Member

@kyranet kyranet commented Oct 21, 2023

👀

Changes

  • Upgraded Yarn from v3 to v4.
  • Downgraded Node.js CI version from 20 to 18 (and pinned to v18 with Volta) because import loader issues in Workers with Node.js v20. See: New --import API breaks worker threads privatenumber/tsx#354
    • Because the loader in v18 can resolve .ts in Worker's constructor, and is required to be passed in the arguments (rather than done at runtime), scripts/workerTsLoader.mjs was deleted.
  • Switched from jest to vitest for better test performance.
  • Switched from ts-node to tsx for better support in Node.js v18 and upwards (uses esbuild under the hood as well). Removed ts-node altogether.
  • Switched from CJS to ESM.
  • Updated discord.js from v13 to v14.
    • Updated discord-api-types/v9 to discord-api-types/v10 to match.
  • Updated @sapphire/framework from v2 to v5.
  • Updated typeorm, giving them a last chance. Skyra now uses less relationships, so it should be probably fine.
  • Split SkyraCommand (extends Subcommand) into SkyraCommand (extends Command) and SkyraSubcommand (extends Subcommand).
  • Removed PaginatedMessageCommand, replaced with Skyra{Subc,C}ommand.PaginatedOptions to reduce code duplication.
  • Removed old v7-* commands. Kept v7-artiel since it's newer and expected to be used soon.
  • Removed unused DB structures (banners and client).
  • Removed left-over API route from the social module (/banners).
  • Removed TriviaManager tests, they're being removed soon in favour of Artiel.

Extras

  • Updated s!conf's menu to listen only to reactions on the menu's message
  • Updated s!dice to default parameter-less to 6
  • Updated getImage to fallback to stickers when there are no attachments
  • Updated channelName argument to throw a less misleading error when filter returns false

Copy link

codecov bot commented Nov 9, 2023

Codecov Report

Attention: 29 lines in your changes are missing coverage. Please review.

Comparison is base (8e9047b) 90.27% compared to head (07215c8) 88.73%.
Report is 5 commits behind head on main.

Files Patch % Lines
src/lib/i18n/translate.ts 63.01% 27 Missing ⚠️
src/lib/database/keys/settings/Channels.ts 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2505      +/-   ##
==========================================
- Coverage   90.27%   88.73%   -1.54%     
==========================================
  Files          88       88              
  Lines        3618     3630      +12     
  Branches      298      281      -17     
==========================================
- Hits         3266     3221      -45     
- Misses        349      403      +54     
- Partials        3        6       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@favna favna left a comment

Choose a reason for hiding this comment

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

I may have made some mistakes in reviewing because there were so many files

.github/workflows/continuous-integration.yml Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
.github/workflows/update-tlds.yml Outdated Show resolved Hide resolved
src/.env.development Show resolved Hide resolved
src/arguments/snowflake.ts Outdated Show resolved Hide resolved
tests/lib/util/functions/emojis/getEmojiObject.test.ts Outdated Show resolved Hide resolved
tests/lib/util.test.ts Outdated Show resolved Hide resolved
tests/mocks/MockInstances.ts Outdated Show resolved Hide resolved
tests/tsconfig.json Outdated Show resolved Hide resolved
vitest.config.ts Show resolved Hide resolved
Copy link
Member

@favna favna left a comment

Choose a reason for hiding this comment

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

Now with request changes

Copy link
Member

@favna favna left a comment

Choose a reason for hiding this comment

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

Couple of comments

tsconfig.base.json Outdated Show resolved Hide resolved
Copy link
Member

@favna favna left a comment

Choose a reason for hiding this comment

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

LGTM

@kyranet kyranet merged commit ee1b6d0 into main Dec 28, 2023
15 checks passed
@kyranet kyranet deleted the refactor/v14 branch December 28, 2023 18:19
kyranet added a commit to sapphiredev/spinel that referenced this pull request Dec 28, 2023
Skyra is now officially on Sapphire v5! skyra-project/skyra#2505
favna pushed a commit to sapphiredev/spinel that referenced this pull request Dec 28, 2023
Skyra is now officially on Sapphire v5! skyra-project/skyra#2505
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants