Skip to content

Build Refactoring #653

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 29 commits into from
Apr 10, 2018
Merged

Build Refactoring #653

merged 29 commits into from
Apr 10, 2018

Conversation

jshcrowthe
Copy link
Contributor

@jshcrowthe jshcrowthe commented Apr 9, 2018

Issues

There are several issues with the build process that I'd like to address:

  • The current build process ignores Typescript compile errors, allowing invalid TS (that fails at release time) to get into the master of the SDK. This is WAI from a typescript viewpoint, so I'm addressing this in some refactoring I've been looking at for a while.
  • The current build process is quite slow
  • The current build process doesn't lend itself to highly optimized use cases

Change Summary

The changes are broken down as follows:

@firebase/* Packages (not including the types packages)

  • Removed gulpfile.js and associated gulp dependencies
  • Added rollup.config.js and associated rollup dependencies (including rollup-plugin-typescript2 which fails builds for TS errors)
  • Updated the main, module, browser fields of each package.json to conform to rollup standards
  • Updated the "scripts" for each package that were referencing gulp to now reference rollup (as pertinent)

@firebase/*-types Packages

  • Removed unused test files and associated test setup

Results

Build Time

Before PR: 130.11s
After PR: 78.30s

Build Size

Before PR

Asset Size
firebase-app.js 35.1 kB
firebase-auth.js 145 kB
firebase-database.js 178 kB
firebase-firestore.js 280 kB
firebase-functions.js 5.85 kB
firebase-messaging.js 26.5 kB
firebase-storage.js 33.7 kB

After PR

Asset Size
firebase-app.js 13 kB
firebase-auth.js 141 kB
firebase-database.js 178 kB
firebase-firestore.js 302 kB
firebase-functions.js 6.8 kB
firebase-messaging.js 33 kB
firebase-storage.js 31 kB

"test": "run-p test:karma type-check lint",
"test:karma": "karma start --single-run",
"test:debug": "karma start --browsers=Chrome --auto-watch",
"prepare": "gulp build",
"prepare": "npm run build",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should these be "yarn build"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we were leveraging yarn as a devDependency here, yes. But since we can't assume yarn exists, sticking w/ NPM is functionally equivalent, and is safer.

@jshcrowthe jshcrowthe changed the title Build Refactoring [DO NOT MERGE] Build Refactoring Apr 10, 2018
@jshcrowthe jshcrowthe merged commit 14c2c4a into master Apr 10, 2018
@jshcrowthe jshcrowthe deleted the build-fixes branch April 10, 2018 23:05
@firebase firebase locked and limited conversation to collaborators Oct 21, 2019
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.

5 participants