Merge branch 'ts-core' of https://github.com/transloadit/uppy into ts…

…-core

* 'ts-core' of https://github.com/transloadit/uppy:
  prettier
  Apply suggestion from code reviews
  fix lint
  Fix unit tests
  meta: fix `js2ts` script on Node.js 20+ (#4802)
  @uppy/companion-client: avoid unnecessary preflight requests (#4462)

.eslintrc.js

@@ -469,7 +469,7 @@ module.exports = {
  },
  {
  files: ['packages/@uppy/*/src/**/*.ts', 'packages/@uppy/*/src/**/*.tsx'],
- excludedFiles: ['packages/@uppy/**/*.test.ts'],
+ excludedFiles: ['packages/@uppy/**/*.test.ts', 'packages/@uppy/core/src/mocks/*.ts'],
  rules: {
  '@typescript-eslint/explicit-function-return-type': 'error',
  },

package.json

@@ -152,7 +152,7 @@
  "test:locale-packs": "yarn locale-packs:unused && yarn locale-packs:warnings",
  "test:locale-packs:unused": "yarn workspace @uppy-dev/locale-pack test unused",
  "test:locale-packs:warnings": "yarn workspace @uppy-dev/locale-pack test warnings",
- "test:unit": "yarn run build:lib && yarn test:watch",
+ "test:unit": "yarn run build:lib && yarn test:watch --run",
  "test:watch": "vitest --environment jsdom --dir packages/@uppy",
  "test": "npm-run-all lint test:locale-packs:unused test:unit test:type test:companion",
  "uploadcdn": "yarn node ./bin/upload-to-cdn.js",

packages/@uppy/companion-client/src/RequestClient.js

@@ -55,9 +55,6 @@ async function handleJSONResponse (res) {
  throw new HttpError({ statusCode: res.status, message: errMsg })
 }
 
-// todo pull out into core instead?
-const allowedHeadersCache = new Map()
-
 export default class RequestClient {
  static VERSION = packageJson.version
 
@@ -84,11 +81,13 @@ export default class RequestClient {
  return stripSlash(companion && companion[host] ? companion[host] : host)
  }
 
- async headers () {
+ async headers (emptyBody = false) {
  const defaultHeaders = {
  Accept: 'application/json',
- 'Content-Type': 'application/json',
- 'Uppy-Versions': `@uppy/companion-client=${RequestClient.VERSION}`,
+ ...(emptyBody ? undefined : {
+ // Passing those headers on requests with no data forces browsers to first make a preflight request.
+ 'Content-Type': 'application/json',
+ }),
  }
 
  return {
@@ -117,88 +116,10 @@ export default class RequestClient {
  return `${this.hostname}/${url}`
  }
 
- /*
- Preflight was added to avoid breaking change between older Companion-client versions and
- newer Companion versions and vice-versa. Usually the break will manifest via CORS errors because a
- version of companion-client could be sending certain headers to a version of Companion server that
- does not support those headers. In which case, the default preflight would lead to CORS.
- So to avoid those errors, we do preflight ourselves, to see what headers the Companion server
- we are communicating with allows. And based on that, companion-client knows what headers to
- send and what headers to not send.
-
- The preflight only happens once throughout the life-cycle of a certain
- Companion-client <-> Companion-server pair (allowedHeadersCache).
- Subsequent requests use the cached result of the preflight.
- However if there is an error retrieving the allowed headers, we will try again next time
- */
- async preflight (path) {
- const allowedHeadersCached = allowedHeadersCache.get(this.hostname)
- if (allowedHeadersCached != null) return allowedHeadersCached
-
- const fallbackAllowedHeaders = [
- 'accept',
- 'content-type',
- 'uppy-auth-token',
- ]
-
- const promise = (async () => {
- try {
- const response = await fetch(this.#getUrl(path), { method: 'OPTIONS' })
-
- const header = response.headers.get('access-control-allow-headers')
- if (header == null || header === '*') {
- allowedHeadersCache.set(this.hostname, fallbackAllowedHeaders)
- return fallbackAllowedHeaders
- }
-
- this.uppy.log(
- `[CompanionClient] adding allowed preflight headers to companion cache: ${this.hostname} ${header}`,
- )
-
- const allowedHeaders = header
- .split(',')
- .map((headerName) => headerName.trim().toLowerCase())
- allowedHeadersCache.set(this.hostname, allowedHeaders)
- return allowedHeaders
- } catch (err) {
- this.uppy.log(
- `[CompanionClient] unable to make preflight request ${err}`,
- 'warning',
- )
- // If the user gets a network error or similar, we should try preflight
- // again next time, or else we might get incorrect behaviour.
- allowedHeadersCache.delete(this.hostname) // re-fetch next time
- return fallbackAllowedHeaders
- }
- })()
-
- allowedHeadersCache.set(this.hostname, promise)
- return promise
- }
-
- async preflightAndHeaders (path) {
- const [allowedHeaders, headers] = await Promise.all([
- this.preflight(path),
- this.headers(),
- ])
- // filter to keep only allowed Headers
- return Object.fromEntries(
- Object.entries(headers).filter(([header]) => {
- if (!allowedHeaders.includes(header.toLowerCase())) {
- this.uppy.log(
- `[CompanionClient] excluding disallowed header ${header}`,
- )
- return false
- }
- return true
- }),
- )
- }
-
  /** @protected */
  async request ({ path, method = 'GET', data, skipPostResponse, signal }) {
  try {
- const headers = await this.preflightAndHeaders(path)
+ const headers = await this.headers(!data)
  const response = await fetchWithNetworkError(this.#getUrl(path), {
  method,
  signal,
@@ -280,7 +201,7 @@ export default class RequestClient {
  || (err.statusCode >= 500 && err.statusCode <= 599 && ![501, 505].includes(err.statusCode))
  )
  if (err instanceof HttpError && !isRetryableHttpError()) throw new AbortError(err);
- 
+
  // p-retry will retry most other errors,
  // but it will not retry TypeError (except network error TypeErrors)
  throw err
@@ -359,7 +280,7 @@ export default class RequestClient {
  socket.send(JSON.stringify({
  action,
  payload: payload ?? {},
- })) 
+ }))
  };
 
  function sendState() {
@@ -379,7 +300,7 @@ export default class RequestClient {
  socketAbortController?.abort?.()
  reject(err)
  }
- 
+
  // todo instead implement the ability for users to cancel / retry *currently uploading files* in the UI
  function resetActivityTimeout() {
  clearTimeout(activityTimeout)
@@ -414,7 +335,7 @@ export default class RequestClient {
 
  try {
  const { action, payload } = JSON.parse(e.data)
- 
+
  switch (action) {
  case 'progress': {
  emitSocketProgress(this, payload, file)
@@ -430,8 +351,8 @@ export default class RequestClient {
  const { message } = payload.error
  throw Object.assign(new Error(message), { cause: payload.error })
  }
-  default:
-  this.uppy.log(`Companion socket unknown action ${action}`, 'warning')
+ default:
+ this.uppy.log(`Companion socket unknown action ${action}`, 'warning')
  }
  } catch (err) {
  onFatalError(err)
@@ -444,7 +365,7 @@ export default class RequestClient {
  if (socket) socket.close()
  socket = undefined
  }
- 
+
  socketAbortController.signal.addEventListener('abort', () => {
  closeSocket()
  })

packages/@uppy/companion/package.json

@@ -63,7 +63,6 @@
  "node-schedule": "2.1.0",
  "prom-client": "14.0.1",
  "redis": "4.2.0",
- "semver": "7.5.3",
  "serialize-error": "^2.1.0",
  "serialize-javascript": "^6.0.0",
  "tus-js-client": "^3.0.0",

packages/@uppy/companion/src/config/companion.js

@@ -18,7 +18,6 @@ const defaultOptions = {
  },
  enableUrlEndpoint: true, // todo next major make this default false
  allowLocalUrls: false,
- logClientVersion: true,
  periodicPingUrls: [],
  streamingUpload: false,
  clientSocketConnectTimeout: 60000,

packages/@uppy/companion/src/server/controllers/connect.js

@@ -19,10 +19,6 @@ module.exports = function connect (req, res) {
  state = oAuthState.addToState(state, { companionInstance: req.companion.buildURL('', true) }, secret)
  }
 
- if (req.companion.clientVersion) {
- state = oAuthState.addToState(state, { clientVersion: req.companion.clientVersion }, secret)
- }
-
  if (req.query.uppyPreAuthToken) {
  state = oAuthState.addToState(state, { preAuthToken: req.query.uppyPreAuthToken }, secret)
  }

packages/@uppy/companion/src/server/middlewares.js

@@ -123,13 +123,13 @@ exports.cors = (options = {}) => (req, res, next) => {
  const exposeHeadersSet = new Set(existingExposeHeaders?.split(',')?.map((method) => method.trim().toLowerCase()))
 
  // exposed so it can be accessed for our custom uppy client preflight
- exposeHeadersSet.add('access-control-allow-headers')
+ exposeHeadersSet.add('access-control-allow-headers') // todo remove in next major, see https://github.com/transloadit/uppy/pull/4462
  if (options.sendSelfEndpoint) exposeHeadersSet.add('i-am')
 
  // Needed for basic operation: https://github.com/transloadit/uppy/issues/3021
  const allowedHeaders = [
  'uppy-auth-token',
- 'uppy-versions',
+ 'uppy-versions', // todo remove in the future? see https://github.com/transloadit/uppy/pull/4462
  'uppy-credentials-params',
  'authorization',
  'origin',
@@ -191,18 +191,12 @@ exports.getCompanionMiddleware = (options) => {
  * @param {Function} next
  */
  const middleware = (req, res, next) => {
- const versionFromQuery = req.query.uppyVersions ? decodeURIComponent(req.query.uppyVersions) : null
  req.companion = {
  options,
  s3Client: getS3Client(options),
  authToken: req.header('uppy-auth-token') || req.query.uppyAuthToken,
- clientVersion: req.header('uppy-versions') || versionFromQuery || '1.0.0',
  buildURL: getURLBuilder(options),
  }
-
- if (options.logClientVersion) {
- logger.info(`uppy client version ${req.companion.clientVersion}`, 'companion.client.version')
- }
  next()
  }
 

packages/@uppy/companion/test/__tests__/callback.js

@@ -43,7 +43,7 @@ describe('test authentication callback', () => {
  test('the token gets sent via html', () => {
  // see mock ../../src/server/helpers/oauth-state above for state values
  return request(authServer)
- .get(`/dropbox/send-token?uppyAuthToken=${token}&state=state-with-newer-version`)
+ .get(`/dropbox/send-token?uppyAuthToken=${token}`)
  .expect(200)
  .expect((res) => {
  const body = `

packages/@uppy/companion/test/mockoauthstate.js

@@ -2,23 +2,11 @@ module.exports = () => {
  return {
  generateState: () => 'some-cool-nice-encrytpion',
  addToState: () => 'some-cool-nice-encrytpion',
- getFromState: (state, key) => {
+ getFromState: (state) => {
  if (state === 'state-with-invalid-instance-url') {
  return 'http://localhost:3452'
  }
 
- if (state === 'state-with-older-version' && key === 'clientVersion') {
- return '@uppy/companion-client=1.0.1'
- }
-
- if (state === 'state-with-newer-version' && key === 'clientVersion') {
- return '@uppy/companion-client=1.0.3'
- }
-
- if (state === 'state-with-newer-version-old-style' && key === 'clientVersion') {
- return 'companion-client:1.0.2'
- }
-
  return 'http://localhost:3020'
  },
  }

packages/@uppy/core/src/Restricter.ts

@@ -1,7 +1,5 @@
 /* eslint-disable max-classes-per-file, class-methods-use-this */
-// @ts-expect-error untyped
 import prettierBytes from '@transloadit/prettier-bytes'
-// @ts-expect-error untyped
 import match from 'mime-match'
 import Translator from '@uppy/utils/lib/Translator'
 import type { Body, Meta, UppyFile } from '@uppy/utils/lib/UppyFile'

packages/@uppy/core/src/UIPlugin.ts

@@ -163,6 +163,7 @@ class UIPlugin<
  * This is the case with @uppy/react plugins, for example.
  */
  render(state: Record<string, unknown>): ComponentChild {
+ // eslint-disable-line @typescript-eslint/no-unused-vars
  throw new Error(
  'Extend the render method to add your plugin to a DOM element',
  )

packages/@uppy/core/src/Uppy.test.ts

@@ -516,7 +516,7 @@ describe('src/Core', () => {
  describe('preprocessors', () => {
  it('should add and remove preprocessor', () => {
  const core = new Core()
- const preprocessor = () => {}
+ const preprocessor = Function.prototype
  expect(core.removePreProcessor(preprocessor)).toBe(false)
  core.addPreProcessor(preprocessor)
  expect(core.removePreProcessor(preprocessor)).toBe(true)
@@ -635,7 +635,7 @@ describe('src/Core', () => {
  describe('postprocessors', () => {
  it('should add and remove postprocessor', () => {
  const core = new Core()
- const postprocessor = () => {}
+ const postprocessor = Function.prototype
  expect(core.removePostProcessor(postprocessor)).toBe(false)
  core.addPostProcessor(postprocessor)
  expect(core.removePostProcessor(postprocessor)).toBe(true)
@@ -752,7 +752,7 @@ describe('src/Core', () => {
  describe('uploaders', () => {
  it('should add and remove uploader', () => {
  const core = new Core()
- const uploader = () => {}
+ const uploader = Function.prototype
  expect(core.removeUploader(uploader)).toBe(false)
  core.addUploader(uploader)
  expect(core.removeUploader(uploader)).toBe(true)
@@ -1326,9 +1326,9 @@ describe('src/Core', () => {
  })
 
  describe('restoring a file', () => {
- it.skip('should restore a file', () => {})
+ it.skip('should restore a file')
 
- it.skip("should fail to restore a file if it doesn't exist", () => {})
+ it.skip("should fail to restore a file if it doesn't exist")
  })
 
  describe('get a file', () => {
@@ -1861,7 +1861,7 @@ describe('src/Core', () => {
  }).not.toThrowError()
  })
 
- it.skip('should enforce the minNumberOfFiles rule', () => {})
+ it.skip('should enforce the minNumberOfFiles rule')
 
  it('should enforce the allowedFileTypes rule', () => {
  const core = new Core({
@@ -2291,6 +2291,8 @@ describe('src/Core', () => {
  data: new File([sampleImage], { type: 'image/jpeg' }),
  })
 
+ // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+ // @ts-ignore accessing private field
  core[Symbol.for('uppy test: createUpload')](
  Object.keys(core.getState().files),
  )
@@ -2312,6 +2314,7 @@ describe('src/Core', () => {
  strings: {
  test: 'beep boop',
  },
+ pluralize: () => 0,
  },
  })