refactor: use fromInnerResponse (nodejs#2635)

* refactor: use fromInnerResponse

* test: add

* Update lib/cache/cache.js

* Update lib/fetch/response.js

* Update request.js

Author: tsctx

lib/cache/cache.js

@@ -3,12 +3,10 @@
 const { kConstruct } = require('./symbols')
 const { urlEquals, fieldValues: getFieldValues } = require('./util')
 const { kEnumerableProperty, isDisturbed } = require('../core/util')
-const { kHeadersList } = require('../core/symbols')
 const { webidl } = require('../fetch/webidl')
-const { Response, cloneResponse } = require('../fetch/response')
+const { Response, cloneResponse, fromInnerResponse } = require('../fetch/response')
 const { Request, fromInnerRequest } = require('../fetch/request')
-const { Headers } = require('../fetch/headers')
-const { kState, kHeaders, kGuard } = require('../fetch/symbols')
+const { kState } = require('../fetch/symbols')
 const { fetching } = require('../fetch/index')
 const { urlIsHttpHttpsScheme, createDeferredPromise, readAllBytes } = require('../fetch/util')
 const assert = require('assert')
@@ -783,11 +781,7 @@ class Cache {
     // 5.5.2
     for (const response of responses) {
       // 5.5.2.1
-      const responseObject = new Response(kConstruct)
-      responseObject[kState] = response
-      responseObject[kHeaders] = new Headers(kConstruct)
-      responseObject[kHeaders][kHeadersList] = response.headersList
-      responseObject[kHeaders][kGuard] = 'immutable'
+      const responseObject = fromInnerResponse(response, 'immutable', { settingsObject: {} })
 
       responseList.push(responseObject.clone())
 

lib/fetch/index.js

@@ -3,13 +3,13 @@
 'use strict'
 
 const {
-  Response,
   makeNetworkError,
   makeAppropriateNetworkError,
   filterResponse,
-  makeResponse
+  makeResponse,
+  fromInnerResponse
 } = require('./response')
-const { Headers, HeadersList } = require('./headers')
+const { HeadersList } = require('./headers')
 const { Request, makeRequest } = require('./request')
 const zlib = require('zlib')
 const {
@@ -45,7 +45,7 @@ const {
   simpleRangeHeaderValue,
   buildContentRange
 } = require('./util')
-const { kState, kHeaders, kGuard, kRealm } = require('./symbols')
+const { kState } = require('./symbols')
 const assert = require('assert')
 const { safelyExtractBody, extractBody } = require('./body')
 const {
@@ -55,7 +55,6 @@ const {
   requestBodyHeader,
   subresourceSet
 } = require('./constants')
-const { kHeadersList, kConstruct } = require('../core/symbols')
 const EE = require('events')
 const { Readable, pipeline } = require('stream')
 const { addAbortListener, isErrored, isReadable, nodeMajor, nodeMinor, bufferToLowerCasedHeaderName } = require('../core/util')
@@ -232,13 +231,7 @@ function fetch (input, init = undefined) {
 
     // 4. Set responseObject to the result of creating a Response object,
     // given response, "immutable", and relevantRealm.
-    responseObject = new Response(kConstruct)
-    responseObject[kState] = response
-    responseObject[kRealm] = relevantRealm
-    responseObject[kHeaders] = new Headers(kConstruct)
-    responseObject[kHeaders][kHeadersList] = response.headersList
-    responseObject[kHeaders][kGuard] = 'immutable'
-    responseObject[kHeaders][kRealm] = relevantRealm
+    responseObject = fromInnerResponse(response, 'immutable', relevantRealm)
 
     // 5. Resolve p with responseObject.
     p.resolve(responseObject)

lib/fetch/request.js

@@ -836,6 +836,7 @@ function cloneRequest (request) {
 }
 
 /**
+ * @see https://fetch.spec.whatwg.org/#request-create
  * @param {any} innerRequest
  * @param {AbortSignal} signal
  * @param {'request' | 'immutable' | 'request-no-cors' | 'response' | 'none'} guard

lib/fetch/response.js

@@ -38,13 +38,8 @@ class Response {
     // The static error() method steps are to return the result of creating a
     // Response object, given a new network error, "immutable", and this’s
     // relevant Realm.
-    const responseObject = new Response(kConstruct)
-    responseObject[kState] = makeNetworkError()
-    responseObject[kRealm] = relevantRealm
-    responseObject[kHeaders] = new Headers(kConstruct)
-    responseObject[kHeaders][kHeadersList] = responseObject[kState].headersList
-    responseObject[kHeaders][kGuard] = 'immutable'
-    responseObject[kHeaders][kRealm] = relevantRealm
+    const responseObject = fromInnerResponse(makeNetworkError(), 'immutable', relevantRealm)
+
     return responseObject
   }
 
@@ -67,13 +62,7 @@ class Response {
     // 3. Let responseObject be the result of creating a Response object, given a new response,
     //    "response", and this’s relevant Realm.
     const relevantRealm = { settingsObject: {} }
-    const responseObject = new Response(kConstruct)
-    responseObject[kState] = makeResponse({})
-    responseObject[kRealm] = relevantRealm
-    responseObject[kHeaders] = new Headers(kConstruct)
-    responseObject[kHeaders][kHeadersList] = responseObject[kState].headersList
-    responseObject[kHeaders][kGuard] = 'response'
-    responseObject[kHeaders][kRealm] = relevantRealm
+    const responseObject = fromInnerResponse(makeResponse({}), 'response', relevantRealm)
 
     // 4. Perform initialize a response given responseObject, init, and (body, "application/json").
     initializeResponse(responseObject, init, { body: body[0], type: 'application/json' })
@@ -111,13 +100,7 @@ class Response {
 
     // 4. Let responseObject be the result of creating a Response object,
     // given a new response, "immutable", and this’s relevant Realm.
-    const responseObject = new Response(kConstruct)
-    responseObject[kState] = makeResponse({})
-    responseObject[kRealm] = relevantRealm
-    responseObject[kHeaders] = new Headers(kConstruct)
-    responseObject[kHeaders][kHeadersList] = responseObject[kState].headersList
-    responseObject[kHeaders][kGuard] = 'immutable'
-    responseObject[kHeaders][kRealm] = relevantRealm
+    const responseObject = fromInnerResponse(makeResponse({}), 'immutable', relevantRealm)
 
     // 5. Set responseObject’s response’s status to status.
     responseObject[kState].status = status
@@ -269,15 +252,7 @@ class Response {
 
     // 3. Return the result of creating a Response object, given
     // clonedResponse, this’s headers’s guard, and this’s relevant Realm.
-    const clonedResponseObject = new Response(kConstruct)
-    clonedResponseObject[kState] = clonedResponse
-    clonedResponseObject[kRealm] = this[kRealm]
-    clonedResponseObject[kHeaders] = new Headers(kConstruct)
-    clonedResponseObject[kHeaders][kHeadersList] = clonedResponse.headersList
-    clonedResponseObject[kHeaders][kGuard] = this[kHeaders][kGuard]
-    clonedResponseObject[kHeaders][kRealm] = this[kHeaders][kRealm]
-
-    return clonedResponseObject
+    return fromInnerResponse(clonedResponse, this[kHeaders][kGuard], this[kRealm])
   }
 }
 
@@ -512,6 +487,24 @@ function initializeResponse (response, init, body) {
   }
 }
 
+/**
+ * @see https://fetch.spec.whatwg.org/#response-create
+ * @param {any} innerResponse
+ * @param {'request' | 'immutable' | 'request-no-cors' | 'response' | 'none'} guard
+ * @param {any} [realm]
+ * @returns {Response}
+ */
+function fromInnerResponse (innerResponse, guard, realm) {
+  const response = new Response(kConstruct)
+  response[kState] = innerResponse
+  response[kRealm] = realm
+  response[kHeaders] = new Headers(kConstruct)
+  response[kHeaders][kHeadersList] = innerResponse.headersList
+  response[kHeaders][kGuard] = guard
+  response[kHeaders][kRealm] = realm
+  return response
+}
+
 webidl.converters.ReadableStream = webidl.interfaceConverter(
   ReadableStream
 )
@@ -588,5 +581,6 @@ module.exports = {
   makeAppropriateNetworkError,
   filterResponse,
   Response,
-  cloneResponse
+  cloneResponse,
+  fromInnerResponse
 }

test/fetch/response.js

@@ -7,10 +7,13 @@ const {
   Response,
   FormData
 } = require('../../')
+const { fromInnerResponse, makeResponse } = require('../../lib/fetch/response')
 const {
   Blob: ThirdPartyBlob,
   FormData: ThirdPartyFormData
 } = require('formdata-node')
+const { kState, kGuard, kRealm, kHeaders } = require('../../lib/fetch/symbols')
+const { kHeadersList } = require('../../lib/core/symbols')
 
 test('arg validation', async () => {
   // constructor
@@ -269,3 +272,19 @@ test('Check the Content-Type of invalid formData', async (t) => {
     await rejects(response.formData(), TypeError)
   })
 })
+
+test('fromInnerResponse', () => {
+  const realm = { settingsObject: {} }
+  const innerResponse = makeResponse({
+    urlList: [new URL('http://asd')]
+  })
+
+  const response = fromInnerResponse(innerResponse, 'immutable', realm)
+
+  // check property
+  assert.strictEqual(response[kState], innerResponse)
+  assert.strictEqual(response[kRealm], realm)
+  assert.strictEqual(response[kHeaders][kHeadersList], innerResponse.headersList)
+  assert.strictEqual(response[kHeaders][kGuard], 'immutable')
+  assert.strictEqual(response[kHeaders][kRealm], realm)
+})