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

chore: fix repeated error message #8153

Merged
merged 1 commit into from
May 13, 2022

Conversation

sapphi-red
Copy link
Member

Description

This PR changes

Failed to resolve entry for package "@cognite/3d-web-parser". The package may have incorrect main/module/exports specified in its package.json: Failed to resolve entry for package "@cognite/3d-web-parser". The package may have incorrect main/module/exports specified in its package.json.

to

Failed to resolve entry for package "@cognite/3d-web-parser". The package may have incorrect main/module/exports specified in its package.json.

which I found while looking at #8141.

Additional context


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@sapphi-red sapphi-red added the p1-chore Doesn't change code behavior (priority) label May 13, 2022
} catch (e) {
packageEntryFailure(id, e.message)
}
packageEntryFailure(id)
Copy link
Member

Choose a reason for hiding this comment

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

How does this work? When there is an exception, won't packageEntryFailure be called twice after this change? And if there isn't an exception, isn't it the same to place the function at the end of the try block or after it 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

packageEntryFailure is essentially a throw new Error().

So the previous behavior is:

  • case (a): no package entry is found
    1. packageEntryFailure(id) is called in L834
    2. an error is thrown inside that
    3. that error is caught by catch in L835
    4. packageEntryFailure(id, e) is called in L836
    5. an error is thrown inside that and gets out of this function
  • case (b): error happened while resolving package entry
    1. error happens inside the try block
    2. that error is caught by catch in L835
    3. packageEntryFailure(id, e) is called in L836
    4. an error is thrown inside that and gets out of this function

the PR behavior is:

  • case (a): no package entry is found
    1. packageEntryFailure(id) is called in L837
    2. an error is thrown inside that and gets out of this function
  • case (b): error happened while resolving package entry
    • this is the same with the current behavior

Copy link
Member

@patak-dev patak-dev May 13, 2022

Choose a reason for hiding this comment

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

Thanks for the explanation! I overlooked the throw error as a log only inside that function. Edit: I overlook the throw twice 😅, double thanks!

@patak-dev patak-dev merged commit 63ec05a into vitejs:main May 13, 2022
@sapphi-red sapphi-red deleted the chore/fix-duplicate-error-message branch May 13, 2022 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p1-chore Doesn't change code behavior (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants