openedx / wg-frontend

Open edX Frontend Working Group
4 stars 0 forks source link

Remove `prop-types` from peer dependencies in our JS libraries #145

Open adamstankiewicz opened 1 year ago

adamstankiewicz commented 1 year ago

In some of our JS repos, we treat prop-types as a peer dependency, when it really shouldn't be (as confirmed by the docs for prop-types itself). Rather, it should be a regular dependency that ships as part of the library.

There's been a couple occasions where prop-types as a peer dependency causes unnecessary peer dependency conflicts that could be avoided had prop-types been considered a regular dependency.

The aforementioned docs on prop-types that provide guidance on how to depend on the package: https://github.com/facebook/prop-types#how-to-depend-on-this-package

AC

ishahroz commented 1 year ago

Hi, I was working on this issue and is blocked by another error I came across while trying to move prop-types from peerDependencies to dependencies. Following are the major steps / observations related to it:

  1. I picked frontend-platform as my JS repo to move prop-types from peerDependencies to dependencies in the package.json file. Simply npm-uninstalled prop-types from peerDependencies (using --save-dev flag) and then installed it again in dependencies context (using --save flag), so that package-lock.json file remains in sync with the changes.
  2. After the above change, frontend-platform build was successful, all the test cases are passing and its front-end preview is also working fine. PR Link
  3. As part of the next step, I tried to test these changes across the frontend-platform's consumer repos. I picked frontend-app-publisher as my consumer repo, created frontend-platform build locally, copied this compressed build in the root directory of frontend-app-publisher and lastly referred and installed frontend-platform local package build by using this notation: "@edx/frontend-platform": "file:edx-frontend-platform-1.0.0-semantically-released.tgz".
  4. Next, when I am trying to test cases for frontend-app-publisher repo, all of them are failing with two error primarily (attached screenshot as the reference):
Screenshot 2022-12-23 at 12 42 59 PM
  1. For the first (cannot find module) error, I found out in the previous version of frontend-platform@2.5.1, the auth module was in the root directory, and later file structure was changed and all the modules were moved into src folder. Simply, changed from '@edx/frontend-platform/auth' to '@edx/frontend-platform/src/auth' in 'src/setupTest.js' and it was referencing it fine.
  2. For the second error, I have tried many applicable solutions related to webpack configuration, browerlist configuration and all of them seem to be not working. Link 1 Link 2.

Even trying the above two workarounds, I am still facing the same issue as what can be seen in the attached screenshot. What are your thoughts on this?