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

Add typescript to worker project #4116

Merged
merged 10 commits into from
Feb 1, 2022
Merged

Add typescript to worker project #4116

merged 10 commits into from
Feb 1, 2022

Conversation

mslourens
Copy link
Contributor

Description

Fixes #4003 - Adds typescript support to the worker project

@mslourens
Copy link
Contributor Author

mslourens commented Jan 20, 2022

I am not sure why the tests fail, because I didn't change actual code, just added typescript and changed the index.js to index.ts. The error message is quite cryptic, but after some debugging I found the actual error:

Error: getaddrinfo ENOTFOUND smtptesthost.com
        at GetAddrInfoReqWrap.onlookup [as oncomplete] (node:dns:71:26)
        at GetAddrInfoReqWrap.callbackTrampoline (node:internal/async_hooks:130:17) {
      errno: -3008,
      code: 'EDNS',
      syscall: 'getaddrinfo',
      hostname: 'smtptesthost.com',
      command: 'CONN'
    }

I tried to compare develop with my branch but didn't see any changes that could cause this error.

Any help is appreciated.

@Rory-Powell
Copy link
Contributor

@mslourens found the issues with the tests, it was to do with changes in how jest mock statements are hoisted when using typescript. The fix has been pushed to your branch. For more context see: kulshekhar/ts-jest#90 (comment)

Copy link
Contributor

@Rory-Powell Rory-Powell left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@codecov-commenter
Copy link

codecov-commenter commented Jan 31, 2022

Codecov Report

Merging #4116 (0716f18) into develop (50bb4a8) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #4116   +/-   ##
========================================
  Coverage    67.95%   67.95%           
========================================
  Files          144      144           
  Lines         4946     4946           
  Branches       762      762           
========================================
  Hits          3361     3361           
  Misses        1117     1117           
  Partials       468      468           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c991f28...0716f18. Read the comment docs.

@mslourens
Copy link
Contributor Author

@Rory-Powell thanks for your help. Somehow I had to build the app before running the e2e tests, it seemed some old version was still being used. Now the tests are green again and we can merge this PR.

@Rory-Powell
Copy link
Contributor

Good call out on the need to build, we had a discussion around this recently and a fix has been added in this PR already
https://github.com/Budibase/budibase/pull/4276/files#diff-a549d2657aa3ea787f685c405385a2c352488058096cb3c4d68a4ab82bf3bf50 so I've just reverted the precy:ci" command in this PR

@shogunpurple shogunpurple merged commit 725c65a into Budibase:develop Feb 1, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Feb 1, 2022
@mslourens mslourens deleted the worker_typescript branch February 10, 2022 08:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add typescript support to worker
4 participants