-
Notifications
You must be signed in to change notification settings - Fork 952
Firestore: Hardcode path names in rollup scripts #7953
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
base: main
Are you sure you want to change the base?
Conversation
….json to improve readability.
|
Size Report 1Affected ProductsNo changes between base commit (991fa27) and merge commit (55d9419).Test Logs |
Size Analysis Report 1Affected ProductsNo changes between base commit (991fa27) and merge commit (55d9419).Test Logs |
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.
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: #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. |
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.