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 @babel/runtime as a dependency #22

Merged
merged 2 commits into from Aug 4, 2020
Merged

🏗 Add @babel/runtime as a dependency #22

merged 2 commits into from Aug 4, 2020

Conversation

RDIL
Copy link
Contributor

@RDIL RDIL commented Jun 30, 2020

Signed-off-by: Reece Dunham [email protected]

Summary

  • Updates some development dependencies
  • Adds @babel/runtime as a dependency

Why

The project currently breaks on Yarn v2 because @babel/plugin-transform-runtime calls @babel/runtime, which is not explicitly declared as one of the project's dependencies.

Checklist

  • Your code builds clean without any errors or warnings
  • You are using approved terminology
  • You have added unit tests, if apply.

@cspotcode
Copy link

Any timeline on when this will be reviewed, merged, and released? I'm coming from facebook/docusaurus#3003

Copy link
Owner

@themgoncalves themgoncalves left a comment

Choose a reason for hiding this comment

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

Hi @RDIL, thanks for contribution, but sadly I cannot accept this pull request as it contains changes outside of the scope.

For that, I recommend you to only add the @babel/runtime into devDependencies (not dependencies itself) and remove all other changes that you made.

Cheers.

@themgoncalves
Copy link
Owner

@cspotcode per commend above.

@RDIL
Copy link
Contributor Author

RDIL commented Jul 27, 2020

@themgoncalves I can revert the other changes, but this does need to be in normal dependencies, because it imports from the @babel/runtime at runtime.

@RDIL RDIL requested a review from themgoncalves July 27, 2020 17:21
@ylemkimon
Copy link

From @babel/plugin-transform-runtime docs:

Make sure you include @babel/runtime as a dependency.

Copy link
Owner

@themgoncalves themgoncalves left a comment

Choose a reason for hiding this comment

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

LGTM

@themgoncalves themgoncalves merged commit 5c07483 into themgoncalves:master Aug 4, 2020
@themgoncalves
Copy link
Owner

@RDIL, @ylemkimon you can find this merge in the just released v0.3.0.

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.

4 participants