Skip to content
This repository has been archived by the owner on Oct 10, 2022. It is now read-only.

chore(deps): update dependency @netlify/eslint-config-node to ^2.2.7 #324

Merged
merged 3 commits into from
Feb 15, 2021
Merged

chore(deps): update dependency @netlify/eslint-config-node to ^2.2.7 #324

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
870e243
chore(deps): update dependency @netlify/eslint-config-node to ^2.2.7
renovate-bot Feb 15, 2021
38dcf49
chore: fix lint errors
erezrokah Feb 15, 2021
f836d60
chore: add todos
erezrokah Feb 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
141 changes: 52 additions & 89 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@
"@babel/plugin-transform-runtime": "^7.12.15",
"@babel/preset-env": "^7.12.16",
"@babel/runtime": "^7.12.13",
"@netlify/eslint-config-node": "^2.2.5",
"@netlify/eslint-config-node": "^2.2.7",
"ava": "^2.4.0",
"babel-loader": "^8.2.2",
"from2-string": "^1.1.0",
Expand Down
4 changes: 3 additions & 1 deletion src/deploy/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,9 @@ const deploySite = async (
phase: 'stop',
})

const uploadList = getUploadList(requiredFiles, filesShaMap).concat(getUploadList(requiredFns, fnShaMap))
const filesUploadList = getUploadList(requiredFiles, filesShaMap)
const functionsUploadList = getUploadList(requiredFns, fnShaMap)
const uploadList = [...filesUploadList, ...functionsUploadList]

await uploadFiles(api, deployId, uploadList, { concurrentUpload, statusCb, maxRetry })

Expand Down
4 changes: 2 additions & 2 deletions src/deploy/upload_files.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ const uploadFiles = async (api, deployId, uploadList, { concurrentUpload, status
})

const uploadFile = async (fileObj, index) => {
const { normalizedPath, assetType, runtime } = fileObj
const readStreamCtor = () => fs.createReadStream(fileObj.filepath)
const { normalizedPath, assetType, runtime, filepath } = fileObj
const readStreamCtor = () => fs.createReadStream(filepath)

statusCb({
type: 'upload',
Expand Down
1 change: 1 addition & 0 deletions src/deploy/util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ const waitForDeploy = async (api, deployId, siteId, timeout) => {
const getUploadList = (required, shaMap) => {
if (!required || !shaMap) return []
// TODO: use `Array.flatMap()` instead once we remove support for Node <11.0.0
// eslint-disable-next-line unicorn/prefer-spread
return [].concat(...required.map((sha) => shaMap[sha]))
Copy link
Contributor

@erezrokah erezrokah Feb 15, 2021 •
edited

Choose a reason for hiding this comment

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

@ehmicky what do think about this change? I don't think array spread can be easily used to flatten arrays like concat.
However on Node.js >=11 Array.flat would be a better choice.

Copy link
Contributor

@ehmicky ehmicky Feb 15, 2021 •
edited

Choose a reason for hiding this comment

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

The suggested change only works when the number of array to flattened is known (as in the examples in the PR). This is not the case for required above.
Specifically, [...required.map(...)] would not flatten arrays.

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like required is an array of strings, not an array of arrays though.

js-client/src/deploy/index.js

Line 104 in 8b01b76

const { id: deployId, required: requiredFiles, required_functions: requiredFns } = deploy

https://github.com/netlify/open-api/blob/master/swagger.yml#L2321

If that's the case, then the following should work:

  return required.map((sha) => shaMap[sha])

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe shaMap is a mapping between a string (sha) to an array of objects.
We would like to get a flattened array of all those objects.
Please correct me if I'm wrong.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, right!
Then we probably need to keep the current code as is.

}

Expand Down
1 change: 1 addition & 0 deletions src/operations.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const omit = require('omit.js').default

// Retrieve all OpenAPI operations
const getOperations = function () {
// eslint-disable-next-line unicorn/prefer-spread
return [].concat(
...Object.entries(paths).map(([path, pathItem]) => {
const operations = omit(pathItem, ['parameters'])
Expand Down