Update package.json deps

[barrymcgee]

Done

Updated gulp-sourcemaps and gulp-sass-lint deps

QA

• Pull code
• Run ./run serve --watch
• Open http://0.0.0.0:8101/vanilla-framework/
• Ensure no errors when running ./run build

[webteam-app]

Starting demo at: http://vanilla-framework-pr-1359.run.demo.haus/vanilla-framework/

[nottrobin]

Was there a reason for updating?

[barrymcgee]

@nottrobin Previously, this error was appearing - bumping these deps should resolve it:

yarn run v0.24.4
$ gulp build
[07:42:16] Using gulpfile /Users/barrymcgee/projects/vanilla-framework/gulpfile.js
[07:42:16] Starting 'lint:sass'...
[07:42:16] Starting 'sass:build'...
[07:42:20] Finished 'lint:sass' after 3.63 s
TypeError: error.setMessage is not a function
    at LazyResult.handleError (/Users/barrymcgee/projects/vanilla-framework/node_modules/postcss/lib/lazy-result.js:162:23)
    at /Users/barrymcgee/projects/vanilla-framework/node_modules/postcss/lib/lazy-result.js:196:27
    at runMicrotasksCallback (internal/process/next_tick.js:64:5)
    at _combinedTickCallback (internal/process/next_tick.js:73:7)
    at process._tickCallback (internal/process/next_tick.js:104:9)
(node:17) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 3): TypeError: error.showSourceCode is not a function
(node:17) DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
Done in 8.85s.

[nottrobin]

Please don't commit package-lock.json, and add it to .gitignore instead. I think it's just asking for trouble to commit it, because it will inevitably get subtly out-of-sync with yarn.lock and then it will look like we're sanctioning two slightly different versions of dependencies. Hopefully Yarn will support package-lock.json someday, then we can revisit this policy.

I should probably write this down in the practices repo.

[barrymcgee]

@nottrobin Done - ready for re-review

[nottrobin]

I'm afraid I'd still seeing the issue with the new version:

[robin@xps] $ ./run build
yarn install v0.24.4
[1/4] Resolving packages...
success Already up-to-date.
Done in 0.80s.
yarn run v0.24.4
$ gulp build 
[11:06:18] Using gulpfile /home/robin/git/vanilla-framework/gulpfile.js
[11:06:18] Starting 'lint:sass'...
[11:06:18] Starting 'sass:build'...
[11:06:20] Finished 'lint:sass' after 1.63 s
TypeError: error.setMessage is not a function
    at LazyResult.handleError (/home/robin/git/vanilla-framework/node_modules/postcss/lib/lazy-result.js:162:23)
    at /home/robin/git/vanilla-framework/node_modules/postcss/lib/lazy-result.js:196:27
    at runMicrotasksCallback (internal/process/next_tick.js:64:5)
    at _combinedTickCallback (internal/process/next_tick.js:73:7)
    at process._tickCallback (internal/process/next_tick.js:104:9)
(node:17) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 3): TypeError: error.showSourceCode is not a function
(node:17) DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
Done in 3.72s.
Configuration file: /home/robin/git/vanilla-framework/_config.yml
            Source: /home/robin/git/vanilla-framework
       Destination: _jekyll/_site
 Incremental build: disabled. Enable with --incremental
      Generating... 
                    done in 0.29 seconds.
 Auto-regeneration: disabled. Use --watch to enable.

Even though I'm definitely using the versions you asked for:

[robin@xps] $ ./run node yarn list | egrep "gulp-sass-lint|gulp-sourcemaps"
├─ @gulp-sourcemaps/identity-map@1.0.1
├─ @gulp-sourcemaps/map-sources@1.0.0
├─ gulp-sass-lint@1.3.3
├─ gulp-sourcemaps@2.6.1
│  ├─ @gulp-sourcemaps/identity-map@1.X
│  ├─ @gulp-sourcemaps/map-sources@1.X

[barrymcgee]

I've isolated this error to gulp-cassnano and raised an issue on the corresponding repo.

[barrymcgee]

@nottrobin Despite this PR not resolving the error above which is now tracked by an issue on this repo, bumping these two deps will bring improvements regardless:

Sass-lint improvements - https://github.com/sasstools/sass-lint/releases/tag/v1.11.0
Fix memory leak in source-maps - https://github.com/gulp-sourcemaps/gulp-sourcemaps/commits/v2.6.1

[nottrobin]

@barrymcgee so now that we live in a world with lockfiles, there's an important difference between what's defined in package.json and what is in yarn.lock (or package-lock.json).

package.json

The versions in package.json are fuzzy, as in for gulp-sourcemaps they say "greater than 2.6.1". Here we're saying, "this app should work with this version onwards".

This allows people who are in a context where they're using the same set of NPM dependencies for a few different purposes (e.g. if they're globally installing dependencies) to be able to sometimes use the version they already have.

Ideally, these should be set at the very oldest version that we know works.

These versions should only be increased when it's necessary to fix an actual error with the project.

yarn.lock

The versions in yarn.lock are explicit versions, and with those we're saying "these are the latest versions we've actually tested it with and we know it works". For developers using ./run, it will always install exactly these versions, and it will also use exactly these versions when building for release to production.

These versions should ideally be updated fairly often, although currently we don't have a process for making sure that happens.

What to do in this case

It seems like you generally want to update a couple of modules, but not to fix any specific bug in vanilla-framework itself. There may be behind-the-scenes improvements, but at the moment we haven't actually seen an issue in vanilla-framework.

It's definitely a good thing to update our modules. But in this case, as vanilla-framework likely works with the old versions, I think you should probably leave all the dependency versions in package.json the same, but make them all fuzzy by adding a ^ in front of them.

Then, you should run ./run node yarn upgrade. This will ask yarn to upgrade all dependencies to their latest versions, and also update yarn.lock.

You should then test the site and build system to make sure nothing seems to have broken, and then commit both package.json (with no versions increased) and yarn.lock (with a ton of version increases).

And we should probably codify this concept of running ./run node yarn upgrade periodically in the practices repo.

[barrymcgee]

@nottrobin Thanks for the comprehensive explanation, I'll do as suggested.

[nottrobin]

Perfect - tested locally, all seems to work. 👍 🎆