Skip to content

refactor(all): use esbuild-runner #1669

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Apr 4, 2022
Merged

refactor(all): use esbuild-runner #1669

merged 8 commits into from
Apr 4, 2022

Conversation

eventualbuddha
Copy link
Collaborator

Overview

Replaces ts-node with esbuild-runner since we weren't using the type checking of ts-node and esbuild-runner is much faster. The catch is that esbuild-runner fails on cycles, so I had to remove the cycles from the repo. I used the import/no-cycle ESLint rule to find them and prevent new ones from forming in the future.

Demo Video or Screenshot

n/a

Testing Plan

Automated testing, manually tested scripts that now use esr instead of ts-node.

Checklist

  • I have added logging where appropriate to any new user actions, system updates such as file reads or storage writes, or errors introduced.
  • I have added JSDoc comments to any newly introduced exports
  • I have added a screenshot and/or video to this PR to demo the change
  • I have added the "user_facing_change" label to this PR to automate an announcement in #machine-product-updates

@render
Copy link

render bot commented Apr 2, 2022

Your Render PR Server URL is https://votingworks-bmd-pr-1669.onrender.com.

Follow its progress at https://dashboard.render.com/static/srv-c946deonlki27rgs1at0.

@eventualbuddha
feat(lint): enable import/no-cycle rule
544c2f2 
Aside from being generally recommended against, it actually breaks `esbuild-runner` if the code it's running has cycles. NodeJS and Webpack handle them fine as long as the mutual dependency is deferred past the initial module load, so it hasn't been a problem yet. But I'd like to move away from `ts-node` and toward `esbuild-runner` because it's much faster, and so we need to get rid of cycles.
@eventualbuddha
refactor(scan): fix cycles in the scan service
0805215
@eventualbuddha
refactor(scan): replace ts-node with esbuild-runner
7a45cb0
@eventualbuddha
fix(scan): remove cycles from ballot-interpreter-nh
36fbebc
@eventualbuddha
fix(ui): remove cycles
06b1d7a
@eventualbuddha
fix(utils): remove cycles
c4769bc
@eventualbuddha
refactor(all): replace ts-node with esbuild-runner
a3be8fe
@eventualbuddha
test(ui): update snapshots
fcde574
benadida
benadida approved these changes Apr 4, 2022
View reviewed changes
Copy link
Contributor

@benadida benadida left a comment

Choose a reason for hiding this comment

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

no cycles feels like a win!

@eventualbuddha eventualbuddha merged commit c1e9ce9 into main Apr 4, 2022
@eventualbuddha eventualbuddha deleted the fix/all/cycles branch April 4, 2022 16:21
@eventualbuddha eventualbuddha mentioned this pull request Apr 5, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benadida benadida benadida approved these changes

@jonahkagan jonahkagan Awaiting requested review from jonahkagan

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@eventualbuddha @benadida