Skip to content
This repository was archived by the owner on Jul 13, 2023. It is now read-only.

Conversation

marbemac
Copy link
Contributor

@marbemac marbemac commented Jun 5, 2019

The parseAuthority hook used to run after the lookup attempt for a pointer, e.g. #/foo. If the target document returned from the authority was not json already, this lookup would fail. This PR makes it so that parseAuthority runs right after the authority target is fetched, before lookup, fixing the problem.

@marbemac marbemac requested a review from P0lip June 5, 2019 01:03
},
"devDependencies": {
"@stoplight/scripts": "5.1.0",
"@stoplight/scripts": "7.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Make sure everything important is exported in the main entry point.

// could not parse... roll back to original value
lookupResult.resolved.result = val;

lookupResult.error = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we leave this error out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The error is still captured as part of the RESOLVE_AUTHORITY error (see change in tests related to this).

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I saw the test was updated, but what if the parseAuthority function throws? Shouldn't we set the former error?

@marbemac marbemac merged commit fb79a2c into master Jun 5, 2019
@marbemac marbemac deleted the fix/authority-parsing branch June 5, 2019 02:14
@stoplight-bot
Copy link
Collaborator

🎉 This PR is included in version 1.5.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

marbemac added a commit that referenced this pull request Jun 26, 2019
This reverts commit fb79a2c.

# Conflicts:
#	package.json
#	yarn.lock
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants