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

Rename npm-shrinkwrap.json to package-lock.json #37493

Merged
merged 3 commits into from
Nov 12, 2019

Conversation

jsnajdr
Copy link
Member

@jsnajdr jsnajdr commented Nov 11, 2019

Rename from the old-fashioned file name to the new one. Was long overdue.

This will invalidate many PR, especially from Mr. Renovate 😐

@jsnajdr jsnajdr added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Nov 11, 2019
@jsnajdr jsnajdr requested review from blowery, sirreal and a team November 11, 2019 19:00
@jsnajdr jsnajdr self-assigned this Nov 11, 2019
@matticbot
Copy link
Contributor

@matticbot
Copy link
Contributor

matticbot commented Nov 11, 2019

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~44150 bytes added 📈 [gzipped])

name                   parsed_size           gzip_size
entry-gutenboarding      +186966 B  (+7.5%)   +44154 B  (+6.9%)
entry-main                   -93 B  (-0.0%)       -1 B  (-0.0%)
entry-login                  -93 B  (-0.0%)       -1 B  (-0.0%)
entry-domains-landing        -93 B  (-0.0%)       -2 B  (-0.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~11 bytes added 📈 [gzipped])

name              parsed_size           gzip_size
gutenberg-editor        +78 B  (+0.0%)      +19 B  (+0.0%)
woocommerce             -18 B  (-0.0%)       -8 B  (-0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~340 bytes removed 📉 [gzipped])

name                                 parsed_size           gzip_size
async-load-lib-happychat-connection       +222 B  (+0.3%)     -340 B  (-1.9%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

"validate-fallback-es5": "npx eslint --env=es5 --no-eslintrc --no-ignore ./public/fallback/*.js",
"postshrinkwrap": "node -e \"fs.utimesSync( './node_modules', new Date(), new Date() );\"",
Copy link
Member

Choose a reason for hiding this comment

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

Rather than removing this, should it be postupdate-deps?

It looks like this may be to prevent us from triggering install-if-deps-outdated after update-deps. Would we lose this with this change or is this redundant?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's right, we need to ensure that after npm run update-deps, the node_modules folder is newer than package-lock.json file. Without the postshrinkwrap code, the package-lock.json file is some 10-15 seconds newer. The install-if-deps-outdated script would reinstall node_modules without it being needed.

I added a touch call to the end of the update-deps script in 18376ed

I'm not sure if using touch (Unix utility) is OK, or if doing that in pure Node.js would be preferred.

@jsnajdr jsnajdr force-pushed the rename/shrinkwrap-to-package-lock branch 2 times, most recently from e230a66 to 18376ed Compare November 12, 2019 10:55
Copy link
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

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

This needs an update here after #37503:

COPY package.json npm-shrinkwrap.json /calypso/

Then I think this is good to go!

@sirreal
Copy link
Member

sirreal commented Nov 12, 2019

This needs an update here after #37503:

COPY package.json npm-shrinkwrap.json /calypso/

Then I think this is good to go!

I've pushed this change.

@jsnajdr jsnajdr force-pushed the rename/shrinkwrap-to-package-lock branch from d37ef0d to 56dbe96 Compare November 12, 2019 11:48
@jsnajdr
Copy link
Member Author

jsnajdr commented Nov 12, 2019

Thanks @sirreal !

@jsnajdr jsnajdr merged commit 41ee2ff into master Nov 12, 2019
@jsnajdr jsnajdr deleted the rename/shrinkwrap-to-package-lock branch November 12, 2019 11:49
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Nov 12, 2019
sirreal added a commit to Automattic/wp-desktop that referenced this pull request Nov 12, 2019
sirreal added a commit to Automattic/wp-desktop that referenced this pull request Nov 14, 2019
* Use Calypso's `package-lock` over `npm-shrinkwrap` for caches

Necessary after calypso PR:
Automattic/wp-calypso#37493

* Update Calypso submodule to [`69dde911636e6f6c8212a06555ce221b0f473832`](Automattic/wp-calypso@69dde91).
loremattei added a commit to Automattic/wp-desktop that referenced this pull request Jan 30, 2020
* Add missing build steps

* CI: Use package-lock over npm-shrinkwrap for caches (#702)

* Use Calypso's `package-lock` over `npm-shrinkwrap` for caches

Necessary after calypso PR:
Automattic/wp-calypso#37493

* Update Calypso submodule to [`69dde911636e6f6c8212a06555ce221b0f473832`](Automattic/wp-calypso@69dde91).

* Add optional chaining and null coalescing support (#707)

* Add optional chaining and null coalescing support

Aligns with Calypso after PR:
Automattic/wp-calypso#37552

* Revert "Update calypso to latest master"

This reverts commit 0143699.

* [deps] Update deps to build with OSX 10.15

* [circle] Remove OSX 10.15 notarization patches

These patches were necessary while waiting to update to a version of
electron-builder that included the code changes necessary to support
the hardened runtime requirement for Mac OS 10.15 notarization
(SHA 7d5f952b12406683fd77a5eaac45e8d6b0b9f257 in electron-builder
repo.)

Confirmed that electron-builder 21.2.0 contains this SHA, so these
patches should be unnecessary.

* Revert electron-updater update

* [REVERT][Makefile] Apply module patches

* [README] Remove manual patch instructions

* [deps] Use own prebuilt spellchecker

In order to pre-empty update of Calypso to Node v12, we need to point electron-spellchecker at our own prebuilt fork which ensures prebuilds are fetched for the current version of Electron and not the environment's version of Node.

* Transpile the chalk module to ES5 (#725)

* [Calypso] Update to latest

* [Makefile] Error on node version mismatch

* [Circle] Use Calypso's .nvmrc in repo root

* [Circle] Update Docker image

* [babel] Polyfill Array.flat

* [Circle] Update Calypso heap/RAM allocation

* [Calypso] Update to latest

* [mac] Add com.apple.quarantine attr

This change makes it so files created by the application (and subprocess)
are quarantined by default. When quarantining files, the system
automatically associates the app name, bundle id with the quarantined
file whenever possible. This mitigates remote-code-execution
vulnerability.

* [version] Bump to 4.7.0-beta1

* Bump version to 4.7.0

Co-authored-by: Jon Surrell <jon.surrell@automattic.com>
Co-authored-by: Nicholas Sakaimbo <nsakaimbo@users.noreply.github.com>
Co-authored-by: Jarda Snajdr <jsnajdr@gmail.com>
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.

3 participants