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

chore: add sourcemaps for JS and minified files #10999 #11012

Merged
merged 5 commits into from
Mar 20, 2018

Conversation

ncoden
Copy link
Contributor

@ncoden ncoden commented Mar 4, 2018

Changes:

  • add sourcemaps for js files (foundation.js and all plugins)
  • add sourcemaps for minified files (foundation.min.js, foundation-*.min.css)
  • improve documentation for dist:javascript gulp task

Related to #10998 (@DanielRuf)
Closes #10999 (@ncoden)

@DanielRuf
Copy link
Contributor

Will check tomorrow.

@ncoden
Copy link
Contributor Author

ncoden commented Mar 19, 2018

Will check tomorrow.

Poke @DanielRuf 😄

.pipe(rename({ suffix: '.min' }))
.pipe(cleancss({ compatibility: 'ie9' }))
.pipe(sourcemaps.write('.', {
mapFile: function(path) { return path.replace('.css.map', '.min.css.map'); }
Copy link
Contributor

@DanielRuf DanielRuf Mar 19, 2018

Choose a reason for hiding this comment

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

This generates .min.min.css.map, should be probably .min.css.map.

Copy link
Contributor Author

@ncoden ncoden Mar 19, 2018

Choose a reason for hiding this comment

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

No because Sourcemaps are initialized before the ".min" renaming to be able retrieve original sourcemaps.

So original sourcemaps xxx.css.map are found because source files are still like xxx.css, and this generate xxx.css.map sourcemaps after the minification we rename to xxx.min.css.map.

CSS sources <--[.css.map]-- .css <--[.css.min.map]-- .min.css

Copy link
Contributor

Choose a reason for hiding this comment

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

image

image

Copy link
Contributor

Choose a reason for hiding this comment

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

Should be

    .pipe(cssFilter)
      .pipe(gulp.dest('./dist/css'))
      .pipe(sourcemaps.init({ loadMaps: true }))
      .pipe(rename({ suffix: '.min' }))
      .pipe(cleancss({ compatibility: 'ie9' }))
      .pipe(sourcemaps.write('.'))
      .pipe(gulp.dest('./dist/css'))
      .pipe(cssFilter.restore)

    .pipe(jsFilter)
      .pipe(gulp.dest('./dist/js'))
      .pipe(sourcemaps.init({ loadMaps: true }))
      .pipe(rename({ suffix: '.min' }))
      .pipe(uglify())
      .pipe(sourcemaps.write('.'))
      .pipe(gulp.dest('./dist/js'));

Copy link
Contributor

Choose a reason for hiding this comment

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

Where there are written does only matter, the initialization collects the needed files. ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm... this worked on my side. I'll check that again. Thanks for the review ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

… task

Sourcemaps output names are resolved at writting once source files are already renamed with the `.min` prefix.
@ncoden
Copy link
Contributor Author

ncoden commented Mar 20, 2018

@DanielRuf Fixed.

@ncoden ncoden merged commit db56c05 into foundation:develop Mar 20, 2018
@ncoden ncoden deleted the chore/and-js-min-sourcefiles-10999 branch April 7, 2018 17:41
@ncoden ncoden added this to the 6.5.0 milestone Apr 23, 2018
ncoden added a commit to ncoden/foundation-sites that referenced this pull request Jun 16, 2018
…files-10999 for v6.5.0

3c5507d chore: add JS sourcemaps
f1bfe51 chore: add sourcemaps for minified files
6f0a602 style: add some doc for minified source map
9b9ed5f fix: remove useless manual sourcemaps prefixing in gulp "deploy:dist" task

Signed-off-by: Nicolas Coden <nicolas@ncoden.fr>
ncoden added a commit to ncoden/foundation-sites that referenced this pull request Jul 9, 2018
Sourcemaps for distribution main files and build JS plugins were added in foundation#11012 but distribution JS plugins were forgotten. This commit change the `deploy:plugins` gulp task to generate and copy sourcemaps for JS plugin alongside their source files.

Changes:
* Split `deploy:plugins` into `deploy:plugins:sources` and `deploy:plugins:sourcemaps`
* Generate sourcemaps for minified plugins in `deploy:plugins:sources`
* Copy sourcemaps for plugins to dist folder in `deploy:plugins:sourcemaps`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants