Skip to content

Conversation

@Rutam21
Copy link
Contributor

@Rutam21 Rutam21 commented Oct 7, 2023

Closes #525

✅ Checklist

  • I have followed every step in the contributing guide
  • The PR title follows the convention.
  • I ran and tested the code works

Testing

Typechecking for the Packages folder has been done as mentioned in the ticket.

pnpm run --filter "./packages/**" typecheck

typecheck

Smoke Testing has been done as suggested by @ericallam .
  • Events and Delays jobs were run using pnpm run events/delay. The commands executed succesfully and the jobs started listening on Port 8080.

delay

  • Then the triggers were run using pnpm run dev trigger that refreshed the job catalog on the Trigger Cloud dashboard.

trigger

  • If any changes were made in the code to either enable or disable a particular job, the trigger kept refreshing the job catalog.

JobRefresh

  • Job were properly listed on the Jon dashboard on Trigger Cloud.

CurrentJobs

  • Jobs were tested on the Cloud Dashboard and were running successfully.

DelayRun

Delay Runs List

  • pnpm run --filter "!./apps/**" typecheck was also run in the root to make sure the types are in-tact.

Root TypeCheck


Changelog

  • All the Packages are updated to use the Node v18.0.0 LTS from now on.
  • Minor fixes are done to support fetch and its available functions.
  • Node-Fetch imports and dependency are removed in all packages and fetch is getting used as the native function as supported by Node 18 now.

Note: Changeset has been added as a Patch release only.

@changeset-bot
Copy link

changeset-bot bot commented Oct 7, 2023

🦋 Changeset detected

Latest commit: 34809d3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 36 packages
Name Type
@trigger.dev/integration-kit Minor
@trigger.dev/eslint-plugin Minor
@trigger.dev/sdk Minor
@trigger.dev/sveltekit Minor
@trigger.dev/express Minor
@trigger.dev/nestjs Minor
@trigger.dev/nextjs Minor
@trigger.dev/astro Minor
@trigger.dev/core Minor
@trigger.dev/cli Minor
@references/nextjs Patch
@trigger.dev/testing Minor
astro-example Patch
@references/job-catalog Patch
nestjs-example Patch
@references/package-tester Patch
@references/remix Patch
svelte-example Patch
@references/unit-testing Patch
perf Patch
@trigger.dev/tailwind-config Minor
@trigger.dev/tsconfig Minor
@trigger.dev/react Minor
@trigger.dev/remix Minor
@trigger.dev/airtable Minor
@trigger.dev/github Minor
@trigger.dev/linear Minor
@trigger.dev/openai Minor
@trigger.dev/plain Minor
@trigger.dev/replicate Minor
@trigger.dev/resend Minor
@trigger.dev/sendgrid Minor
@trigger.dev/slack Minor
@trigger.dev/stripe Minor
@trigger.dev/supabase Minor
@trigger.dev/typeform Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Rutam21
Copy link
Contributor Author

Rutam21 commented Oct 7, 2023

@ericallam Please review and suggest changes, if any. Thanks.

@ericallam
Copy link
Member

At first glance this is looking good. Will try and find time over the weekend to review but will get to it by Monday latest 👍

@ericallam
Copy link
Member

Pulled this down and looks like there is an issue with the pnpm-lock.yaml file as I'm getting this error when running pnpm install:

Ignoring broken lockfile at /Users/eric/code/triggerdotdev/trigger.dev: Lockfile /Users/eric/code/triggerdotdev/trigger.dev/pnpm-lock.yaml not compatible with current pnpm

Can you make sure you are using [email protected]

@Rutam21
Copy link
Contributor Author

Rutam21 commented Oct 9, 2023

@ericallam I have updated the pnpm.lock file as per pnpm v7.18.1. My curent system pnpm is also set to 7.18.1.

pnpm version

Please review and suggest changes, if any. Thank you.

@ericallam
Copy link
Member

Unfortunately because of the original usage of pnpm v8, when you ran pnpm install with pnpm v7 in this branch it caused a complete re-creation of the pnpm-lock.yaml file, resulting in massive changes to the dependencies that are installed (basically as if there was no pnpm-lock.yaml file to begin with). To be able to merge this PR more quickly I would suggest reverting all changes made to the pnpm-lock.yaml file from the beginning of these PR and re-run pnpm install, starting from the last known good point. One way you could do that is grabbing the contents of pnpm-lock.yaml from the commit just before your commits and copy/pasting into your local pnpm-lock.yaml file and then running pnpm install and then committing/pushing.

@Rutam21
Copy link
Contributor Author

Rutam21 commented Oct 9, 2023

@ericallam Thanks for the suggestion. I grabbed the lock file from the main branch of my fork from which this PR branch was created and then did a pnpm i and pushed the updated the lock file back. Hope this resolves all the previous version conflicts.

@ericallam ericallam merged commit 975c5f1 into triggerdotdev:main Oct 10, 2023
arjunindia pushed a commit to arjunindia/trigger.dev that referenced this pull request Oct 10, 2023
… node-fetch (triggerdotdev#581)

* [TRI-1333] : Upgrade all packages to use Node18 and use fetch instead of node-fetch

* Changeset Added

* Update pnpm-lock to compatible pnpm v7.18.1

* Restored pnpm-lock to the last known good point compatible with pnpm v7.18.1

* Update chilled-plants-exercise.md

---------

Co-authored-by: Eric Allam <[email protected]>
@Rutam21 Rutam21 deleted the TRI-1333 branch October 19, 2023 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[TRI-1333] [TRI-1332] Upgrade all packages/* and use fetch instead of node-fetch

2 participants