firebase
Firestore: Hardcode path names in rollup scripts
This PR modifies Firestore's rollup scripts to hardcode the paths of the input and output files, rather than loading them from package.json. No functionality whatsoever was changed. The motivation for this change was to improve readability of the rollup scripts: with the paths hardcoded it is much easier to understand from reading the scripts which files are getting consumed and which are getting created.
Work In Progress
This PR is a work-in-progress. As of Jan 22, 2024, work on this PR has been de-prioritized but I'm leaving it here so it can be picked up in the future. Googlers see b/302682954 for details.
⚠️ No Changeset found
Latest commit: fd3855db2c93302bd7d8d86315b037480e367c3f
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
![changeset-bot[bot] avatar](https://avatars.githubusercontent.com/in/28616?v=4 "Published by a pub.dev verified publisher"){kind=small}
Size Report 1
Affected Products
No changes between base commit (991fa27) and merge commit (55d9419).Test Logs
- Base (991fa27): https://github.com/firebase/firebase-js-sdk/actions/runs/7537086987
- Merge (55d9419): https://github.com/firebase/firebase-js-sdk/actions/runs/7538421365
Size Analysis Report 1
Affected Products
No changes between base commit (991fa27) and merge commit (55d9419).Test Logs
- Base (991fa27): https://github.com/firebase/firebase-js-sdk/actions/runs/7537086987
- Merge (55d9419): https://github.com/firebase/firebase-js-sdk/actions/runs/7538421365
I'm leaning towards the risks outweighing the readability improvement. Two downsides. First, I think this makes the code more fragile, requiring changes to the paths to be made consistently in multiple places in code. Second, this makes Firestore inconsistent with other packages. The inconsistency makes it more likely that a Firebase maintainer would not modify all appropriate strings. I'm open to discussing.
Thank you for the input, @MarkDuckworth. Based on these comments I'm trying a different approach that meets somewhere in the middle. I'll let you know once that's ready.
@MarkDuckworth How do you feel about this solution instead: https://github.com/firebase/firebase-js-sdk/pull/7956
Basically, it puts the logic to extract the paths from package.json into a helper function so that we don't have all these bespoke mechanisms for doing so scattered throughout the code.