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

Clean up Options. #841

Closed
NTillmann opened this issue Jul 26, 2017 · 2 comments
Closed

Clean up Options. #841

NTillmann opened this issue Jul 26, 2017 · 2 comments
Labels
help wanted refactoring

Comments

@NTillmann
Copy link
Contributor

Look at options.js. There are a few things that should get cleaned up:

  1. Options should really be just the union type of RealmOptions and SerializerOptions. Why is there the silly name-difference between errorHandler and onError?
  2. The Options data type (and associated helper functions) should not be defined in options.js, move them to a file called prepack-options.js, which is not referenced by the prepack core engine. This is currently violated by src/environment.js in the executePartialEvaluator function. It should probably take a new PartialEvaluatorOptions types, which only contains sourceMaps.
@NTillmann NTillmann changed the title Clean up Options. Clean up Options. Jul 26, 2017
@echo304
Copy link
Contributor

echo304 commented Aug 9, 2017

I'm gonna tackle this issue ✋

@echo304 echo304 mentioned this issue Aug 9, 2017
Closed
facebook-github-bot pushed a commit that referenced this issue Aug 11, 2017
@echo304 @facebook-github-bot
Clean up options and fix test code
1bbba37 
Summary:
- Clean up options and create new `prepack-options.js` file
- Add `PartialEvaluatorOptions` type which contains only `sourceMaps`
- Fix test code related to above change

#841
Closes #878

Reviewed By: NTillmann

Differential Revision: D5605347

Pulled By: cblappert

fbshipit-source-id: 024284a9904dff765417c2c849c4642e781affb3
@NTillmann
Copy link
Contributor Author

Fixed by merged pull request.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
help wanted refactoring
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@NTillmann @echo304