feat(build)!: use tsup #54

c43721 · 2022-02-03T00:06:03Z

Fixes #50

Build goes from 2 seconds to 20ms. Also cleans dist each time you run build.

Breaking changes:
uses esbuild to build instead of tsc
~~dev command no longer registers commands. It will auto-recompile + restart the bot if changes are made.~~

MarcusOtter · 2022-02-03T01:17:57Z

This looks super cool, I'll make sure to give tsup a proper read through and test this out before merging, but awesome work!

dev command no longer registers commands

This is actually pretty important since the commands won't update and you can't try the bot otherwise (if the commands are being developed). How do you think we should solve this? 🤔 Hot reloading seems awesome, I just wish there was a way we could have the best of both worlds.

I guess one very hacky thing I have in mind is to read what the current commands are and store that in a json file, then every hot reload it will read that object and see if anything needs to be changed, and if so deploy the new changes to the development guild.

c43721 · 2022-02-03T01:36:49Z

Keeping track of command data is super annoying, so I forwent hot reloading commands since it could lead to API absue (think- you change a typo on a command then started to work on logic for that command).

We'd have to do something like sapphire/framework does and auto register based on what API responds. That's doable but requires custom code, not something I wish to do.

c43721 · 2022-02-03T02:03:48Z

alright technically I can copy/paste their code but would mean that we'd have 2 licensed since they use MIT

MarcusOtter · 2022-02-03T02:11:39Z

I don't think that matters @c43721

From MIT:

Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.

As long as we keep the copyright & permission notice in the file, we are allowed to sublicense the code.

More info: https://softwareengineering.stackexchange.com/questions/105912/can-you-change-code-distributed-under-the-mit-license-and-re-distribute-it-unde

c43721 · 2022-02-03T02:14:22Z

shows how much I know about licenses

c43721 · 2022-02-05T18:10:58Z

this now registers commands on startup, but does not hot reload. There's a deploy:dev command to deploy development commands from scripts (since dev registers once + watches build)

…into MarcusOtter-main

c43721 · 2022-02-22T04:45:39Z

never make me merge changes again...
@MarcusOtter Is there a chance this could get a review this week? @nchristopher anything changes you would like by chance?

n1ckoates · 2022-02-22T04:54:09Z

package.json

-    "deploy": "npm run undeploy && node ./scripts/deploy-commands.js --global"
+    "build": "tsup",
+    "start": "npm run build && node --enable-source-maps .",
+    "dev": "npm run deploy:dev && tsup --watch --onSuccess \"node --enable-source-maps .",


This string isn't terminated, resulting in the below error when executing npm run dev.

> discord-needle@1.0.0 dev > npm run deploy:dev && tsup --watch --onSuccess "node --enable-source-maps . sh: 1: Syntax error: Unterminated quoted string

Suggested change

"dev": "npm run deploy:dev && tsup --watch --onSuccess \"node --enable-source-maps .",

"dev": "npm run deploy:dev && tsup --watch --onSuccess \"node --enable-source-maps .\"",

MarcusOtter · 2022-02-22T09:20:58Z

never make me merge changes again...

oops 😄 I can fix the conflicts and review this week hopefully, I was unsure about the status of this PR which is why I haven't touched it

c43721 · 2022-02-22T23:26:06Z

Good for a review now, FYI.

n1ckoates

It seems that config.json is being built as config.js, which causes imports to fail.

c43721 · 2022-03-06T00:42:51Z

it should work :/

I didn't test this so I'll take another look eventually

c43721 · 2022-03-06T01:25:17Z

@MarcusOtter Has merge conflicts :(

n1ckoates · 2022-03-06T01:34:30Z

/scripts/deploy-commands.js needs to be modified to only use .js files otherwise it will attempt to deploy commands from .map.

MarcusOtter · 2022-03-06T01:35:50Z

/scripts/deploy-commands.js needs to be modified to only use .js files otherwise it will attempt to deploy commands from .map.

I did this already with #100
https://github.com/MarcusOtter/discord-needle/pull/100/files#diff-4b6f85f2408aa620bf4897cb7a2f87fd4ee9fc30d8e36cc46bb4184e5681a5e9L61-R62

c43721 · 2022-03-06T01:36:38Z

tsconfig cannot have baseUrl for this to work

n1ckoates · 2022-03-06T01:38:50Z

@c43721 Your fork's branch needs to be updated in order to fix command deploys.

tsup.config.ts

src/helpers/configHelpers.ts

…into feat/use-esbuild

n1ckoates · 2022-03-06T03:16:12Z

src/commands/configure.ts

@@ -16,7 +16,7 @@
 // ________________________________________________________________________________________________

 import { SlashCommandBuilder } from "@discordjs/builders";
-import { ChannelType } from "discord-api-types";
+import { ChannelType } from "discord-api-types/v9";


This change should be done everywhere else we import discord-api-types.

We should. I would like to do this in a follow up though

This should be done in #94.

MarcusOtter · 2022-03-13T22:58:54Z

I'm not sure what the state of this is, should we turn it into a draft for now? Does it need my review? Last thing I heard was something about fixing imports, otherwise builds break, does that still apply?

c43721 · 2022-04-16T16:44:22Z

Something something JSON.. I'll close since this is so far behind and I'll re-open/

feat(build)!: use tsup

fb2a27a

fix: address issues with cp and dev not deploying changes

11a2077

c43721 and others added 4 commits February 6, 2022 09:00

Merge branch 'MarcusOtter:main' into feat/use-esbuild

7fa8680

fix(build): fix merge conflicts

44019d7

Merge branch 'main' of https://github.com/MarcusOtter/discord-needle …

56c5cf0

…into MarcusOtter-main

Merge branch 'MarcusOtter-main' into feat/use-esbuild

e1c9ae2

feat(build): enable node source maps

ab9b239

n1ckoates reviewed Feb 22, 2022

View reviewed changes

fix: Same as last commit with changes

ed5f5d9

n1ckoates suggested changes Feb 23, 2022

View reviewed changes

fix: fix funny stuff

f907107

fix: deploy commands on start

06adad6

n1ckoates suggested changes Mar 6, 2022

View reviewed changes

tsup.config.ts Outdated Show resolved Hide resolved

src/helpers/configHelpers.ts Show resolved Hide resolved

c43721 added 2 commits March 5, 2022 19:45

fix o__O

d723be4

Merge branch 'main' of https://github.com/MarcusOtter/discord-needle …

0c9cbcb

…into feat/use-esbuild

c43721 added 4 commits March 5, 2022 19:52

fix: maps

1c1d037

undo

07fc4bd

fix: import from v9

bf037a6

fix: no deploy :(

52dcff1

n1ckoates suggested changes Mar 6, 2022

View reviewed changes

MarcusOtter marked this pull request as draft March 21, 2022 20:02

c43721 closed this Apr 16, 2022

c43721 deleted the feat/use-esbuild branch June 25, 2022 01:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(build)!: use tsup #54

feat(build)!: use tsup #54

c43721 commented Feb 3, 2022 •

edited

Loading

MarcusOtter commented Feb 3, 2022 •

edited

Loading

c43721 commented Feb 3, 2022

c43721 commented Feb 3, 2022

MarcusOtter commented Feb 3, 2022 •

edited

Loading

c43721 commented Feb 3, 2022

c43721 commented Feb 5, 2022

c43721 commented Feb 22, 2022

n1ckoates Feb 22, 2022 •

edited

Loading

c43721 Feb 22, 2022

MarcusOtter commented Feb 22, 2022

c43721 commented Feb 22, 2022

n1ckoates left a comment

c43721 commented Mar 6, 2022

c43721 commented Mar 6, 2022

n1ckoates commented Mar 6, 2022

MarcusOtter commented Mar 6, 2022 •

edited

Loading

c43721 commented Mar 6, 2022

n1ckoates commented Mar 6, 2022

n1ckoates Mar 6, 2022

c43721 Mar 6, 2022

n1ckoates Mar 6, 2022

MarcusOtter commented Mar 13, 2022

c43721 commented Apr 16, 2022

	"dev": "npm run deploy:dev && tsup --watch --onSuccess \"node --enable-source-maps .",
	"dev": "npm run deploy:dev && tsup --watch --onSuccess \"node --enable-source-maps .\"",

feat(build)!: use tsup #54

feat(build)!: use tsup #54

Conversation

c43721 commented Feb 3, 2022 • edited Loading

MarcusOtter commented Feb 3, 2022 • edited Loading

c43721 commented Feb 3, 2022

c43721 commented Feb 3, 2022

MarcusOtter commented Feb 3, 2022 • edited Loading

c43721 commented Feb 3, 2022

c43721 commented Feb 5, 2022

c43721 commented Feb 22, 2022

n1ckoates Feb 22, 2022 • edited Loading

Choose a reason for hiding this comment

c43721 Feb 22, 2022

Choose a reason for hiding this comment

MarcusOtter commented Feb 22, 2022

c43721 commented Feb 22, 2022

n1ckoates left a comment

Choose a reason for hiding this comment

c43721 commented Mar 6, 2022

c43721 commented Mar 6, 2022

n1ckoates commented Mar 6, 2022

MarcusOtter commented Mar 6, 2022 • edited Loading

c43721 commented Mar 6, 2022

n1ckoates commented Mar 6, 2022

n1ckoates Mar 6, 2022

Choose a reason for hiding this comment

c43721 Mar 6, 2022

Choose a reason for hiding this comment

n1ckoates Mar 6, 2022

Choose a reason for hiding this comment

MarcusOtter commented Mar 13, 2022

c43721 commented Apr 16, 2022

c43721 commented Feb 3, 2022 •

edited

Loading

MarcusOtter commented Feb 3, 2022 •

edited

Loading

MarcusOtter commented Feb 3, 2022 •

edited

Loading

n1ckoates Feb 22, 2022 •

edited

Loading

MarcusOtter commented Mar 6, 2022 •

edited

Loading