Clean up options and fix test code #878
Conversation
echo304
changed the title
src/prepack-options.js
Outdated
| import type { ErrorHandler } from "./errors.js"; | ||
| import type { SerializerOptions, RealmOptions } from "./options"; | ||
|
|
||
| export type Compatibility = "browser" | "jsc-600-1-4-17" | "node-source-maps" | "node-cli"; |
There was a problem hiding this comment.
Don't redefine, import from "./options";
src/prepack-options.js
Outdated
| import type { SerializerOptions, RealmOptions } from "./options"; | ||
|
|
||
| export type Compatibility = "browser" | "jsc-600-1-4-17" | "node-source-maps" | "node-cli"; | ||
| export const CompatibilityValues = ["browser", "jsc-600-1-4-17", "node-source-maps", "node-cli"]; |
src/prepack-options.js
Outdated
| return { | ||
| compatibility, | ||
| debugNames, | ||
| errorHandler: errorHandler, |
There was a problem hiding this comment.
simplify to errorHandler,
src/prepack-options.js
Outdated
| let result: SerializerOptions = { | ||
| delayInitializations, | ||
| delayUnsupportedRequires, | ||
| initializeMoreModules: speculate, |
There was a problem hiding this comment.
Another silly naming discrepancy that should be addressed eventually. If that's harmonized, then the Options datatype could be truly the union SerializerOptions | RealmOptions | PartialEvaluatorOptions, right?
src/prepack-options.js
Outdated
| export type Compatibility = "browser" | "jsc-600-1-4-17" | "node-source-maps" | "node-cli"; | ||
| export const CompatibilityValues = ["browser", "jsc-600-1-4-17", "node-source-maps", "node-cli"]; | ||
|
|
||
| export type Options = {| |
There was a problem hiding this comment.
You could rename this type to PrepackOptions, or CombinedPrepackOptions, to make it a bit more clear what this type represents.
There was a problem hiding this comment.
let's pick PrepackOptions
|
Awesome! @cblappert, do you think this might break any internal tooling? |
|
@cblappert has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
prepack-options.jsfilePartialEvaluatorOptionstype which contains onlysourceMaps#841