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 swc CLI #19

Merged
merged 3 commits into from
Feb 25, 2021
Merged

Refactor swc CLI #19

merged 3 commits into from
Feb 25, 2021

Conversation

kherock
Copy link
Contributor

@kherock kherock commented Feb 19, 2021

This consolidates most of the logic within the dir and file modes of the CLI, around 100 lines were able to be removed. Due to Node 10's EOL in April and there being places where flatMap and fs.rmdir(..., { recursive: true }) are helpful, I've bumped the minimum Node version to v12. Let me know if it's an issue to be dropping Node 10 support a bit prematurely.

This closes both #11 and #17. This also provides a simple fix to swc-project/swc#1255 (I've documented the root cause as a comment referencing swc-project/swc#1388).

I noticed 3 options on the CliOptions interface that had no corresponding commander option that I removed:

  • verbose
  • keepFileExtension
  • relative

Aside from that, I thought it was strange behavior to have --quiet prevent errors from writing to stderr. It already had no influence over the unhandled promise rejection handler, so now errors will log in a few more places regardless of the presence of --quiet. Additionally, I made it so that file deletions in directory watch mode will delete output files when they exist. Aside from these, no new behavior was added.

@kdy1
Copy link
Member

kdy1 commented Feb 20, 2021

I'll review this tomorrow. (Sorry for delay)

Copy link
Member

@kdy1 kdy1 left a comment

Choose a reason for hiding this comment

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

LGTM. Great work! 😊

But I'm a bit worried about the version of dependencies.

package.json Outdated Show resolved Hide resolved
@kherock
Copy link
Contributor Author

kherock commented Feb 21, 2021

Thank you!

And are you referring to Node 10 support or just some of the other package dependencies (I know commander is several major versions behind)? I should have time to address those if you need me to.

@kdy1
Copy link
Member

kdy1 commented Feb 21, 2021

@kherock I meant the versions of dependencies.

@kherock
Copy link
Contributor Author

kherock commented Feb 22, 2021

@kdy1 I upgraded the remaining dependencies and pinned the peer dependency for chokidar. I should be done here!

@kdy1
Copy link
Member

kdy1 commented Feb 25, 2021

Thanks!

@kdy1 kdy1 merged commit bc78609 into swc-project:master Feb 25, 2021
@kdy1
Copy link
Member

kdy1 commented Feb 25, 2021

I tried publishing but it resulted in an error.

@kherock
Copy link
Contributor Author

kherock commented Feb 25, 2021

Do you have any details I can help resolve? I'm not able to reproduce any publishing errors. I just attempted to publish a the package under my own npm scope and it seems to be working fine: https://www.npmjs.com/package/@kherock/swc-cli

@kdy1
Copy link
Member

kdy1 commented Feb 25, 2021

@kherock Sorry. I reinstalled node dependencies and it works.
Thanks!

@kherock kherock deleted the fix-sourcemaps branch February 25, 2021 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants