-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix: include ajv in devDependencies for Expo Router and npm #2842
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
fix: include ajv in devDependencies for Expo Router and npm #2842
Conversation
|
16e30eb is the more targeted approach, but we can easily drop that commit if simple is preferred. |
frankcalise
left a comment
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.
Thanks for this! I think being more specific here is fine with the npm+router check
Does something need a proper upstream fix somewhere?
|
Yeah, |
|
@coolsoftwaretyler ah yeah I had a discussion about this in Slack. config-standard is abandoned I think for neostandard in that issue thread https://infinitered.slack.com/archives/C0X0P5LP7/p1730325079888599 should we look to swap it out for neo and see if n just works or cut back on all these extra config / plugins? |
|
@frankcalise - if we swap in neostandard and it Just Works, I think that's ideal. I can take a look sometime this week to make sure it doesn't cause any changes while linting a freshly ignited project. Want to merge this one for now to patch up the |
## [10.0.5](v10.0.4...v10.0.5) (2024-11-27) ### Bug Fixes * **boilerplate:** remove interop definition for FlashList ([#2838](#2838) by [@frankcalise](https://github.com/frankcalise)) ([b368bcb](b368bcb)) * **boilerplate:** update app.tsx useEffect ([#2850](#2850) by [@objective](https://github.com/objective)See) ([2886487](2886487)) * **cli:** component generator template off in expo-router ([#2854](#2854) by [@frankcalise](https://github.com/frankcalise)) ([95f7642](95f7642)) * **cli:** include ajv in devDependencies for Expo Router and npm ([#2842](#2842) by [@coolsoftwaretyler](https://github.com/coolsoftwaretyler)) ([6baf76b](6baf76b))
|
🎉 This PR is included in version 10.0.5 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Please verify the following:
yarn testjest tests pass with new tests, if relevantyarn linteslint checks pass with new code, if relevantyarn format:checkprettier checks pass with new code, if relevantREADME.md(or relevant documentation) has been updated with your changesDescribe your PR
This would resolve one of the issues in #2840, although there are additional problems with the Expo Router template.
This is a little bit of a sledgehammer approach, but I wanted to start with the simple solution and we can enhance it if that seems necessary.
In #2823, we added the
--legacy-peer-depsflag fornpm installcommands. This fixed a problem upstream from standard/eslint-config-standard#412, but I believe it has opened up a new problem for users who choose to use Expo Router and npm. Based on the issues that Expo Router + ESLint has experienced with how bun resolves peer dependencies, I am guessing the legacy peer dependency resolution falls victim to the same problem.The very simplest way to solve this is if the generated Ignite project includes
ajvat the right version number in its owndevDependencies. This version "wins" across the board.The drawback to this approach is that if Expo Router changes its required
ajvversion, there may be some new errors for the project to resolve.Another drawback to this PR is that it would add
ajvto all projects, including other package managers that don't suffer from this problem, and including projects that do not opt in to Expo Router. So we could alternatively runnpm install ajv@^8 --legacy-peer-depsin the install command right afterignite/src/tools/packager.ts
Line 149 in e59d880
Screenshots (if applicable)
No screenshots, but running
node ~/ignite/bin/ignite new project-nameand choosingnpmand Expo Router successfully works with this change.