Skip to content
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

Add dev and prod build options #1353

Closed
wants to merge 1 commit into from
Closed

Add dev and prod build options #1353

wants to merge 1 commit into from

Conversation

Hirse
Copy link
Contributor

@Hirse Hirse commented May 7, 2020

This PR adds a flag to the grunt build to switch between dev and prod mode.
The build is changed to compress and optimize the less and js bundles, buts adds inline source maps to the files in dev mode.

File size comparison after including #1350, #1351, and #1352 comparing to latest release:

                 1.5.7     prod     dev   
styles.css        191kb    129kb     474kb
ungit.js         4882kb    500kb    3506kb
components        357kb     96kb     665kb
component js      344kb     85kb     624kb
component css      13kb     11kb      41kb

Note, that css only have sourcemaps in the new dev build.

I tried to create source maps as separate files to be loaded when required, but couldn't get it to work correctly with less. (Source map path has to adapted to future server serving location.)

Running the browserify optimizations (from the tinyify plugin) increases the time that task takes significantly. For development, we might want to look into better watch-options, for example watchify, which in turn might be difficult to integrate into the current grunt workflow, especially since we are not using any grunt-browserify. (Longer term, a shift from grunt to npm scripts might be the best way. It already seems to be the way the community is going.)

@campersau @jung-kim @wmertens Would love some feedback on this.

@campersau
Copy link
Collaborator

Hm I am not sure about removing source maps in PROD. At least for javascript they are quite helpful because they would include the original filename and such in the stack trace. See #1333
#1330 (comment) for recent issues which included the client side log.

So I would only disable tinyify in DEV and enable it in PROD and leave source maps always on.

@Hirse
Copy link
Contributor Author

Hirse commented May 8, 2020

Having the source maps inlined adds 3mb (a 600% increase) to the ungit.js bundle. I don't know how much we care about that since most users run on localhost anyway, but I would consider that significant.

We could investigate further whether having external source maps (i.e. in .map files) is the best solution. I don't know if they improve the stack trace sufficiently.

The rationale for minifying (i.e. running tinyify) in DEV mode was to have more comparable builds and prevent errors from originating from the build process itself (like dependencies that get optimized out in PROD builds).

@wmertens
Copy link
Contributor

wmertens commented May 9, 2020

In prod mode I always replace source maps with links, and depending on the project I serve them or not. I also use an error reporting endpoint that maps the errors using the sourcemaps.

Sending minified files for prod use is very useful, it really cuts down on load time. Parsing 3MB of javascript takes a long time.

Can you configure tinify to replace semicolons with returns? Uglify can do this, and the resulting file is equally small but it has way shorter lines, which makes debugging the minified code much simpler.

@campersau
Copy link
Collaborator

External sourcemaps are now working for both js and css files. See #1394

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants