Skip to content

Make Firestore-mangled build the only Firestore build #2640

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

Merged
merged 4 commits into from
Feb 19, 2020

Conversation

schmidt-sebastian
Copy link
Contributor

@schmidt-sebastian schmidt-sebastian commented Feb 18, 2020

This removes the .min.js files for Firestore and updates the default build to use the minified property names.

It looks like the previous build did not include a mangled Node build - I added this here and because of that I also had to add externs for Node.

@@ -49,6 +46,7 @@
"@firebase/app-types": "0.x"
},
"devDependencies": {
"@types/node": "8.10.59",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is in devDependencies in the root package.json, is there a reason to include it separately here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The paths in externs.json are relative to the package root, so I need this to be installed in packages/firestore/node_modules.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't line 38 of firestore/rollup.config.js set them to be relative to repo root? The other files in externs.json seem to be relative to repo root.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! Updated.

@schmidt-sebastian
Copy link
Contributor Author

@Feiyang1 Let me know if you have time to check whether this works properly with SourceMaps.

@schmidt-sebastian
Copy link
Contributor Author

@Feiyang1 confirmed the source maps still work as is. Thanks!

@schmidt-sebastian schmidt-sebastian merged commit 261bbd5 into master Feb 19, 2020
@firebase firebase locked and limited conversation to collaborators Mar 21, 2020
@schmidt-sebastian schmidt-sebastian deleted the mrschmidt/minifieddefault branch March 26, 2020 03:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants