department-of-veterans-affairs / va-mobile-library

https://department-of-veterans-affairs.github.io/va-mobile-library/
ISC License
1 stars 0 forks source link

Design System - CU - Explore Improved Components Package Publish Config #136

Open TimRoe opened 11 months ago

TimRoe commented 11 months ago

Proposed Change

While working on department-of-veterans-affairs/va-mobile-app#6989, I came across yarn documentation on the package.json that appears to allow you to override the main file that the project kicks off from within the published package. In early stages of the components package, we wanted this exact thing but (I believe) did not discover this option and instead figured out a workaround way to diverge the logic flow to separate the expo/Storybook flow run for testing within the VA Mobile Library repo and what is actually run when packaged up for consumption by an outside app using try/catch based on the existing of the App.tsx file and leveraging .npmignore. Despite working and allowing simplicity of no tooling to "just work" both for development and when published to NPM, this is a messy and not intuitive solution to any future engineers that happen upon it.

Why Should We Prioritize?

If this alternative works, it would make the structure of the components package much more streamlined and intuitive to any future developers and any external teams that had a reason to look under the hood.

Coding Time Estimation

Gave a 5--if this new option works as expected, should be fairly straightforward clean-up of the current solution, but bumped higher to account for it requiring cutting likely a decent amount of alpha test builds and running within the VA Mobile App to validate everything is working as expected without breaking existing component(s) in any way.

Testing Considerations

No QA testing for this ticket.

Draft testing should include:

Acceptance Criteria

While this should not be a breaking change, this should be treated as one since it could completely break the components package:

TimRoe commented 9 months ago

The core of this ticket did not pan out as expected. Setting a publishConfig.main within the components package did not prevent the base main file from inclusion with the published package despite the file being in .npmignore which in turn causes a crash attempting to use the package within the flagship app due to not having the appropriate dependencies, as occurred prior to the workaround this ticket intended to streamline.

I created a Q&A discussion item in the yarn repo to determine if this is a bug or just the way it behaves for a reason. If it's a bug, this ticket will be iceboxed until fixed; if working as intended it can be closed out.

To document, there's a similar sounding outstanding yarn bug here and an exactly matching bug with pnpm here lending to behavioral expectations being correct and it being a yarn bug to also not work.

TimRoe commented 9 months ago

Moving to Icebox as the sprint is ending and no response has yet been made to the Q&A discussion item. I have subscribed to that and the yarn bug to receive email notifications if/when any follow-up occurs.