Workspace base rtt #21793
Workspace base rtt #21793
Conversation
babel.config.json
Outdated
| @@ -0,0 +1,111 @@ | |||
| { | |||
| // Javascript settings. | |||
There was a problem hiding this comment.
Normally the JSON format doesn't allow comments, which I think is why these comments are highlighted in red
There was a problem hiding this comment.
Normally the JSON format doesn't allow comments, which I think is why these comments are highlighted in red
that makes sense. we had to change the extension for workspaces.
package.json
Outdated
| @@ -304,7 +304,7 @@ | |||
| "mapbox-gl": "^1.12.0", | |||
| "markdown-it": "^12.3.2", | |||
| "markdown-it-link-attributes": "^4.0.0", | |||
| "moment": "~2.24.0", | |||
| "moment": "^2.29.2", | |||
There was a problem hiding this comment.
Any potential side effects from this upgrade? If so I'd recommend doing it in a separate PR
There was a problem hiding this comment.
Same. unless this is required, let's move it to a new PR
script/delete-modules-f.sh
Outdated
| @@ -0,0 +1,24 @@ | |||
| # get argument or default to "." | |||
There was a problem hiding this comment.
I'm not immediately sure what the -f means in this filename. Consider removing that, or expanding it to be more descriptive (-force?)
There was a problem hiding this comment.
The f was meant to signify folder, but update this file and script name accordingly
| "**/cached-path-relative": "1.1.0", | ||
| "**/moment": "^2.29.2", |
There was a problem hiding this comment.
Was this removed on purpose? I assume we still need it in there, no?
There was a problem hiding this comment.
"**/yargs-parser": "13.1.2"
"punycode": "1.4.1"
"**/moment": "^2.29.2"
We removed these because otherwise Yarn would attempt to alter the .lockfile (even when the weren't any dependency changes in the package.json), in turn blocking CI
There was a problem hiding this comment.
without removing these listings in the resolutions field yarn install --frozen-lockfile throws an error: error Your lockfile needs to be updated, but yarn was run with '--frozen-lockfile'.
There was a problem hiding this comment.
Right, but I'm pretty sure these are in there for a reason. Usually security issues. Please confirm before merging
There was a problem hiding this comment.
This is the PR from @jhonnyoddball that set the moment resolution: #20768 (high severity security issue)
Webpack 5 upgrade brought in punycode (also @jhonnyoddball ) #19403
There was a problem hiding this comment.
Here's more background on the punycode/IE issue: mathiasbynens/punycode.js#86
There was a problem hiding this comment.
Yeah, it was there because of security vulnerability
There was a problem hiding this comment.
We will need to find a work around it
There was a problem hiding this comment.
With the latest changes in this PR, looking at the yarn.lock, I see:
"moment@>= 2.9.0", moment@^2.22.1:
version "2.29.4"
and
[email protected], [email protected], punycode@^1.3.2, punycode@^2.1.0, punycode@^2.1.1:
version "1.4.1"
We should be fine with resolving to those versions. I would only be concerned if I saw yarn.lock changing to resolve to an older version of moment (security issues) or a newer version of punycode (IE issues)
| "@babel/runtime": "^7.15.4", | ||
| "punycode": "1.4.1", |
There was a problem hiding this comment.
Previously we had to downgrade punycode because newer versions of punycode caused errors in IE 11. Make sure to test thoroughly in IE 11
…s-affairs/vets-website into workspace-base-RTT
Description
This PR adds workspaces to vets-website
Original issue(s)
department-of-veterans-affairs/va.gov-team#39205
Testing done
local testing passed. project built successfully; browser testing in local, dev, staging, prod , RI environments.
Screenshots
Acceptance criteria
Definition of done