Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is transform overload unnecessary? #3089

Closed
zoeyzhao19 opened this issue Apr 28, 2023 · 3 comments
Closed

Is transform overload unnecessary? #3089

zoeyzhao19 opened this issue Apr 28, 2023 · 3 comments

Comments

@zoeyzhao19
Copy link

zoeyzhao19 commented Apr 28, 2023

esbuild transform api has an overload type declaration

export declare function transform<SpecificOptions extends TransformOptions>(input: string | Uint8Array, options?: SpecificOptions): Promise<TransformResult<SpecificOptions>>
export declare function transform(input: string | Uint8Array, options?: TransformOptions): Promise<TransformResult>

Which I might think it respect custom options extended from TransformOptions, But when I pass my option which is extended from TransformOptions,

transform('function(){console.log("Hello World")}', {
  loader: 'ts',
  customOptionFlag: true,
})

My custom option could not pass and it reported Error: Invalid option in transform() call: "customOptionFlag".
and my ts check does not report the wrong type. Since esbuild does not handle user's custom transform option, so I doubt that whether is this transform overload necessary.

@hyrious
Copy link

hyrious commented Apr 28, 2023

If you look at the TransformResult definition, it changes the mangleCache and legalComments fields according to the input type:

export interface TransformResult<SpecificOptions extends TransformOptions = TransformOptions> {
  code: string
  map: string
  warnings: Message[]
  /** Only when "mangleCache" is present */
  mangleCache: Record<string, string | false> | (SpecificOptions['mangleCache'] extends Object ? never : undefined)
  /** Only when "legalComments" is "external" */
  legalComments: string | (SpecificOptions['legalComments'] extends 'external' ? never : undefined)
}

So, it makes sure the result object has mangleCache or legalComments when your input options set them.

let r = await esbuild.transform('', {})
r.mangleCache   // record | undefined
r.legalComments // string | undefined

let t = await esbuild.transform('', {
  mangleCache: {},
  legalComments: 'external',
})
r.mangleCache   // record
r.legalComments // string

@zoeyzhao19
Copy link
Author

@hyrious , But mangleCache and legalComments are already optional fields in TransformOptions (extended from CommonOptions) decalration

@hyrious
Copy link

hyrious commented Apr 28, 2023

@zoeyzhao19 Please look carefully. They are optional if you don't pass in mangleCache, but will be non-optional when you pass one. So that code below will not cause type error:

let t = await esbuild.transform('', {
  mangleCache: { someProp_: false },
  legalComments: 'external',
})
let comments = t.legalComments.trim()
// because t.legalComments must be string, so calling .trim() on it is ok

@evanw evanw closed this as completed in 70b22b1 May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants