-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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/core dependencies to react/preact integrations #3928
Conversation
🦋 Changeset detectedLatest commit: e06f7d5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
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 |
@@ -31,6 +31,7 @@ | |||
"dev": "astro-scripts dev \"src/**/*.ts\"" | |||
}, | |||
"dependencies": { | |||
"@babel/core": ">=7.0.0-0 <8.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@babel/core
is only used inside of astro, so I think adding this to a peerDependencies" instead of
dependencies` would also silence the error but not result in adding this as an explicit dependency (since pnpm/yarn support bubbling up peer dependencies this way).
Worth confirming, but I think that's the better move here if it works!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooh, small hiccup with this approach, astro add
automatically installs any peerDependencies
from these packages into your Astro project. :(
Not sure the best way to handle this...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried it and didn't work unfortunately. I think by making it a peerDependency
you are making the user install it.
└─┬ @astrojs/react
├── ✕ missing peer @babel/core@">=7.0.0-0 <8.0.0"
└─┬ @babel/plugin-transform-react-jsx
├── ✕ missing peer @babel/core@^7.0.0-0
└─┬ @babel/plugin-syntax-jsx
└── ✕ missing peer @babel/core@^7.0.0-0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, that's surprising since Astro has it as a dependency. I actually thought that having it as a dep in astro solved this, so maybe it's new behavior from pnpm?
I guess I'm not against leaving it in deps then!
Changes
@astrojs/react
and@astrojs/preact
depend on@babel/plugin-transform-react-jsx
which as@babel/core
as a peerDependencies.@babel/core
as a dependency, fixing the issue.Testing
N/A
Docs
N/A