Skip to content
This repository was archived by the owner on Aug 4, 2021. It is now read-only.

Conversation

keithamus
Copy link
Contributor

@keithamus keithamus commented Aug 3, 2018

This is a simple refactoring to make main code less boilerplatey. It removes the new Promise invocation instead Promisifying the resolve package to make the main function easier to read (less indents!)

diff without whitespace

Copy link
Contributor

@shellscape shellscape left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Structurally, this looks good to me. I can't yet comment on any potential side effects (not there yet with my knowledge of the codebase as yet)

Copy link
Contributor

@guybedford guybedford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems completely semantically equivalent to me. Thanks for the refactoring!

.then(resolved => {
if (options.browser && packageBrowserField) {
if (packageBrowserField[ resolved ]) {
resolved = packageBrowserField[ resolved ];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is odd for the error case, but I guess it was already doing that!

@guybedford guybedford merged commit 05bd0ef into master Aug 3, 2018
@keithamus keithamus deleted the refactor-promisify-resolveid-for-less-promise-boilerplate branch August 3, 2018 17:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants