Update dependencies that use minimist #3363

thisisdano · 2020-03-17T18:13:55Z

This does not mitigate use of minimist throughout our dependencies, but it gets us a little closer. This also does a couple things:

~~Fixes an output bug for the accordion component~~
~~Removes nswatch and replaces it with gulp watch~~

[EDIT: NO LONGER DOES THOSE THINGS]

mejiaj

We need to fix watch task error'ing out on linting errors and minor question on accordion.

mejiaj · 2020-03-18T14:29:29Z

gulpfile.js

@@ -71,3 +71,9 @@ gulp.task("default", function(done) {

  done();
 });
+
+gulp.task("watch", function() {
+  gulp.watch("src/stylesheets/**/*.scss", gulp.series("sass")),


Don't we need to lint sass/js as well?

Right now if I add a selector with an ID on src/stylesheets/packages/_uswds-components.scss for example I get:

Error

[09:29:10] The following tasks did not complete: watch, <series>, sass, <anonymous> [09:29:10] Did you forget to signal async completion? events.js:288 throw er; // Unhandled 'error' event ^ NodeError: write after end at writeAfterEnd (/Users/jamesmejia/web/uswds/node_modules/streamfilter/node_modules/readable-stream/lib/_stream_writable.js:257:12) at StreamFilter.Writable.write (/Users/jamesmejia/web/uswds/node_modules/streamfilter/node_modules/readable-stream/lib/_stream_writable.js:301:21) at DestroyableTransform.ondata (/Users/jamesmejia/web/uswds/node_modules/readable-stream/lib/_stream_readable.js:619:20) at DestroyableTransform.emit (events.js:311:20) at DestroyableTransform.EventEmitter.emit (domain.js:505:15) at addChunk (/Users/jamesmejia/web/uswds/node_modules/readable-stream/lib/_stream_readable.js:291:12) at readableAddChunk (/Users/jamesmejia/web/uswds/node_modules/readable-stream/lib/_stream_readable.js:278:11) at DestroyableTransform.Readable.push (/Users/jamesmejia/web/uswds/node_modules/readable-stream/lib/_stream_readable.js:245:10) at DestroyableTransform.Transform.push (/Users/jamesmejia/web/uswds/node_modules/readable-stream/lib/_stream_transform.js:148:32) at Pumpify.onReadable (/Users/jamesmejia/web/uswds/node_modules/to-through/index.js:25:14) Emitted 'error' event on StreamFilter instance at: at StreamFilter.onerror (/Users/jamesmejia/web/uswds/node_modules/readable-stream/lib/_stream_readable.js:640:52) at StreamFilter.emit (events.js:311:20) at StreamFilter.EventEmitter.emit (domain.js:482:12) at writeAfterEnd (/Users/jamesmejia/web/uswds/node_modules/streamfilter/node_modules/readable-stream/lib/_stream_writable.js:259:10) at StreamFilter.Writable.write (/Users/jamesmejia/web/uswds/node_modules/streamfilter/node_modules/readable-stream/lib/_stream_writable.js:301:21) [... lines matching original stack trace ...] at readableAddChunk (/Users/jamesmejia/web/uswds/node_modules/readable-stream/lib/_stream_readable.js:278:11) npm ERR! code ELIFECYCLE npm ERR! errno 1 npm ERR! uswds@2.5.1 watch: `NODE_ENV=development gulp watch` npm ERR! Exit status 1 npm ERR! npm ERR! Failed at the uswds@2.5.1 watch script. npm ERR! This is probably not a problem with npm. There is likely additional logging output above. npm ERR! A complete log of this run can be found in: npm ERR! /Users/jamesmejia/.npm/_logs/2020-03-18T14_29_10_110Z-debug.log npm ERR! code ELIFECYCLE npm ERR! errno 1 npm ERR! uswds@2.5.1 start: `fractal start --sync & npm run watch` npm ERR! Exit status 1 npm ERR! npm ERR! Failed at the uswds@2.5.1 start script. npm ERR! This is probably not a problem with npm. There is likely additional logging output above. npm ERR! A complete log of this run can be found in: npm ERR! /Users/jamesmejia/.npm/_logs/2020-03-18T14_29_10_137Z-debug.log

Maybe? I was just trying to duplicate the nswatch script:

"watch": { "src/stylesheets/**/*.scss": "build:css", "src/js/**/*.js": "build:js" },

"build:css": "gulp sass", "build:js": "gulp javascript",

afaik, we rely on npm run test for linting and haven't been running it on js compile

↻ Checking this on current develop branch.

Okay, it works in the old watch task, which watches for all changes on all scss files.

line 38: "src/stylesheets/**/*.scss": "build:css",

~~Versus the gulp watch in sass.js~~

98: return gulp .src("src/stylesheets/uswds.scss")

My mistake, I see the new watch task:

gulp.watch("src/stylesheets/**/*.scss", gulp.series("sass")),

Looking into this.

reverted to nswatch for the time being

mejiaj · 2020-03-18T15:48:43Z

src/components/06-accordion/accordion.njk

-  </h2>
-  <div id="{{ accordion.id_prefix }}{{ item.id }}" class="usa-accordion__content usa-prose">
-    {{ item.content | safe }}
+{% for item in accordion.items %}


Does this fix the Multiselectable in USWDS-Site? I don't think a loop is necessary here, unless we wanted to output them all on a single page. I'm using alerts.config.yml and alerts.njk for reference.

{% for item in accordion.items %} is for the individual accordion items.
This is what I get for accordion variables with {{ accordion | dump }}

{{ accordion | dump(2) }}

{ "items": [ { "title": "First Amendment", "id": "a1", "expanded": true, "content": "Congress shall make no law respecting an establishment of religion, or prohibiting the free exercise thereof; or abridging the freedom of speech, or of the press; or the right of the people peaceably to assemble, and to petition the Government for a redress of grievances.\n" }, { "title": "Second Amendment", "id": "a2", "content": "A well regulated Militia, being necessary to the security of a free State, the right of the people to keep and bear Arms, shall not be infringed. <ul><li>This is a list item</li><li>Another list item</li></ul>\n" }, { "title": "Third Amendment", "id": "a3", "content": "No Soldier shall, in time of peace be quartered in any house, without the consent of the Owner, nor in time of war, but in a manner to be prescribed by law.\n" }, { "title": "Fourth Amendment", "id": "a4", "content": "The right of the people to be secure in their persons, houses, papers, and effects, against unreasonable searches and seizures, shall not be violated, and no Warrants shall issue, but upon probable cause, supported by Oath or affirmation, and particularly describing the place to be searched, and the persons or things to be seized.\n" }, { "title": "Fifth Amendment", "id": "a5", "content": "No person shall be held to answer for a capital, or otherwise infamous crime, unless on a presentment or indictment of a Grand Jury, except in cases arising in the land or naval forces, or in the Militia, when in actual service in time of War or public danger; nor shall any person be subject for the same offence to be twice put in jeopardy of life or limb; nor shall be compelled in any criminal case to be a witness against himself, nor be deprived of life, liberty, or property, without due process of law; nor shall private property be taken for public use, without just compensation.\n" } ], "id_prefix": "m-", "multiselectable": true }

Site doesn't need a fix for multiselectable since it doesn't document or demo this functionality yet. (Maybe it should.) This fixes an issue where all the accordions are wrapped in a single usa-accordion — look at the code on https://designsystem.digital.gov/components/accordion/ — it's wrapping all the accordions in a single usa-accordion where each one should get its own.

nm — I guess the whole group should be wrapped in a single usa-accordion — I'll revert

mejiaj · 2020-03-18T15:54:59Z

package.json

    "test": "snyk test && gulp eslint && gulp typecheck && gulp stylelint && gulp test && gulp regression",
    "test:ci": "gulp eslint && gulp typecheck && gulp stylelint && gulp test && gulp regression",
    "test:unit": "gulp test",
    "test:visual": "node ./spec/visual-regression-tester.js test",
    "test:visual:update": " node ./spec/visual-regression-tester.js test --updateGolden",
    "version": "gulp build release",
-    "watch": "NODE_ENV=development nswatch"
+    "watch": "NODE_ENV=development gulp watch"
  },
  "watch": {
    "src/stylesheets/**/*.scss": "build:css",


I think lines 37-40 are for nswatch.

Yeah, I should get rid of them

Actually, I reverted to nswatch

thisisdano added 8 commits March 17, 2020 09:55

Ignore system tokens with prettier

1b7adf9

Update stylelint

da19c9d

Update gulp

cf2f886

Replace nswatch with gulp watchfile

20dbfc9

Update stylelint-related packages

41f1ccb

Use parallel execution

13556bb

Loop around usa-accordion, not inside it

771b425

npm audit fix

6a3106a

thisisdano marked this pull request as ready for review March 17, 2020 19:46

Update package-lock.json

99c18a6

thisisdano requested a review from mejiaj March 18, 2020 14:17

mejiaj requested changes Mar 18, 2020

View reviewed changes

mejiaj reviewed Mar 18, 2020

View reviewed changes

thisisdano added 2 commits March 18, 2020 09:12

Revert to keeping all related accordion items in a single usa-accordion

ebb45ed

Reinstate nswatch :(

2d2c16b

mejiaj approved these changes Mar 18, 2020

View reviewed changes

Remove extra indent in accordion component.

2602560

thisisdano merged commit c84d88b into develop Mar 18, 2020

thisisdano deleted the dw-fix-minimist branch March 18, 2020 18:14

thisisdano mentioned this pull request Mar 18, 2020

Update NPM packages. #3334

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update dependencies that use minimist #3363

Update dependencies that use minimist #3363

thisisdano commented Mar 17, 2020 •

edited

mejiaj left a comment

mejiaj Mar 18, 2020

thisisdano Mar 18, 2020

thisisdano Mar 18, 2020

mejiaj Mar 18, 2020

mejiaj Mar 18, 2020 •

edited

thisisdano Mar 18, 2020

mejiaj Mar 18, 2020

thisisdano Mar 18, 2020

thisisdano Mar 18, 2020

thisisdano Mar 18, 2020

mejiaj Mar 18, 2020

thisisdano Mar 18, 2020

thisisdano Mar 18, 2020

Update dependencies that use minimist #3363

Update dependencies that use minimist #3363

Conversation

thisisdano commented Mar 17, 2020 • edited

mejiaj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mejiaj Mar 18, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thisisdano commented Mar 17, 2020 •

edited

mejiaj Mar 18, 2020 •

edited