Skip to content

Build Tools: Validate package-lock.json for "resolved" errors#22237

Merged
aduth merged 3 commits intomasterfrom
add/validate-lockfile
May 11, 2020
Merged

Build Tools: Validate package-lock.json for "resolved" errors#22237
aduth merged 3 commits intomasterfrom
add/validate-lockfile

Conversation

@aduth
Copy link
Copy Markdown
Member

@aduth aduth commented May 8, 2020

Related Slack discussion (link requires registration): https://wordpress.slack.com/archives/C02QB2JS7/p1588692036240600
Related upstream issue: npm/cli#1138
Previously:

This pull request seeks to add a pre-commit and continuous integration build step for validating the contents of package-lock.json. Notably, this is intended to try to capture issues where an invalid resolved value becomes introduced in package-lock.json. The script bin/validate-package-lock.js will deeply check the contents of the lockfile for any resolved: false.

The idea for this change proposal was inspired by @adamziel 's comment at #21873 (comment).

Testing Instructions:

Introduce "resolved": false in a dependency in package-lock.json (manually) and attempt to commit the change. Observe an error.

Found invalid resolved dependency in package-lock.json.

{
	"semver": {
		"version": "5.7.1",
		"resolved": false,
		"integrity": "sha512-sauaDf/PZdVgrLTNYHRtpXa1iRiKcaebiKQ1BJdpQlWH2lCvexQdX55snPFyK7QzpudqbCI0qXFfOasHdyNDGQ=="
	}
}

To fix, try removing the node_modules directory, then run npm install.

@aduth aduth added the [Type] Build Tooling Issues or PRs related to build tooling label May 8, 2020
@aduth aduth requested a review from adamziel May 8, 2020 18:33
@aduth aduth requested review from ajitbohra, nerrad and ntwb as code owners May 8, 2020 18:33
@aduth aduth mentioned this pull request May 8, 2020
Merged
5 tasks
jorgefilipecosta
jorgefilipecosta approved these changes May 8, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Nice verification, I verified the error triggers when expected.

@jorgefilipecosta
Copy link
Copy Markdown
Member

jorgefilipecosta commented May 8, 2020

Unfortunately it seems on travis we get "bin/validate-package-lock.js(19,30): error TS2307: Cannot find module '../package-lock'.", so the json file is not read as expected.

@aduth
Copy link
Copy Markdown
Member Author

aduth commented May 8, 2020

@jorgefilipecosta Yeah, funny enough, quite similar to very recent discussion with @sirreal at #19866 (comment) . I think a @ts-ignore might be appropriate here as well.

@jorgefilipecosta
Copy link
Copy Markdown
Member

jorgefilipecosta commented May 8, 2020

On the wp-scripts package we read JSON files with:

	const data = fs.readFileSync( fileName, 'utf8' );
	return JSON.parse( data );

Maybe using this approach works on travis?

@github-actions
Copy link
Copy Markdown

github-actions bot commented May 8, 2020

Size Change: +89 B (0%)

Total Size: 824 kB

Filename Size Change
build/block-editor/index.js 102 kB +108 B (0%)
build/block-library/index.js 115 kB -19 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 4.08 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.61 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 761 B 0 B
build/block-editor/style-rtl.css 10.3 kB 0 B
build/block-editor/style.css 10.3 kB 0 B
build/block-library/editor-rtl.css 7.12 kB 0 B
build/block-library/editor.css 7.12 kB 0 B
build/block-library/style-rtl.css 7.34 kB 0 B
build/block-library/style.css 7.34 kB 0 B
build/block-library/theme-rtl.css 683 B 0 B
build/block-library/theme.css 685 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.1 kB 0 B
build/components/index.js 180 kB 0 B
build/components/style-rtl.css 17 kB 0 B
build/components/style.css 16.9 kB 0 B
build/compose/index.js 6.66 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.45 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 3.1 kB 0 B
build/edit-navigation/index.js 4.41 kB 0 B
build/edit-navigation/style-rtl.css 618 B 0 B
build/edit-navigation/style.css 617 B 0 B
build/edit-post/index.js 28 kB 0 B
build/edit-post/style-rtl.css 12.2 kB 0 B
build/edit-post/style.css 12.2 kB 0 B
build/edit-site/index.js 12.1 kB 0 B
build/edit-site/style-rtl.css 5.22 kB 0 B
build/edit-site/style.css 5.22 kB 0 B
build/edit-widgets/index.js 8.37 kB 0 B
build/edit-widgets/style-rtl.css 4.69 kB 0 B
build/edit-widgets/style.css 4.69 kB 0 B
build/editor/editor-styles-rtl.css 425 B 0 B
build/editor/editor-styles.css 428 B 0 B
build/editor/index.js 44.3 kB 0 B
build/editor/style-rtl.css 5.07 kB 0 B
build/editor/style.css 5.08 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 734 B 0 B
build/format-library/index.js 7.63 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 2.14 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 14.8 kB 0 B
build/server-side-render/index.js 2.68 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.02 kB 0 B
build/viewport/index.js 1.84 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.18 kB 0 B

compressed-size-action

@jorgefilipecosta
Copy link
Copy Markdown
Member

jorgefilipecosta commented May 9, 2020

Travis is green I guess we can merge :)

@aduth aduth merged commit 263fefc into master May 11, 2020
@aduth aduth deleted the add/validate-lockfile branch May 11, 2020 12:34
@github-actions github-actions bot added this to the Gutenberg 8.1 milestone May 11, 2020
@adamziel
Copy link
Copy Markdown
Contributor

adamziel commented May 11, 2020

🎉

@gziolo
Copy link
Copy Markdown
Member

gziolo commented May 12, 2020

Nice one, thanks for working on it 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jorgefilipecosta jorgefilipecosta jorgefilipecosta approved these changes

@adamziel adamziel Awaiting requested review from adamziel

@ajitbohra ajitbohra Awaiting requested review from ajitbohra

@nerrad nerrad Awaiting requested review from nerrad

@ntwb ntwb Awaiting requested review from ntwb

Assignees

No one assigned

Labels

[Type] Build Tooling Issues or PRs related to build tooling

Projects

None yet

Milestone

Gutenberg 8.1

Development

Successfully merging this pull request may close these issues.

4 participants

@aduth @jorgefilipecosta @adamziel @gziolo