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

fix: skip possible api route when ENOENT #7242

Closed
wants to merge 1 commit into from

Conversation

leoj3n
Copy link
Contributor

@leoj3n leoj3n commented Mar 9, 2022

Description

image

Before PR:

image

After PR:

image

Additional context

Related file/line:

if (e.code !== 'ENOENT') {
throw e
}

Not sure if this is the right location and code to fix this but it does resolve the issue in current state.

Not sure if test is needed or where would be added.

What is the purpose of this pull request?

  • Bug fix

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.

@patak-dev
Copy link
Member

Thanks for the PR. For the moment, I don't think we can proceed with it. We will move away from suffixes once some proposals advance. This is not only an issue for raw but for other suffixes too. For the moment, I think the best is to add a middleware in your project to handle these routes before they get to Vite's pipeline. See https://vitejs.dev/guide/api-plugin.html#configureserver

@patak-dev patak-dev closed this Mar 12, 2022
@leoj3n
Copy link
Contributor Author

leoj3n commented Mar 13, 2022

@patak-dev Thanks for the explanation. Can you point to the specific proposals? Interested to take a look.

@patak-dev
Copy link
Member

See this issue #7017, there are links from there to the diff proposals.

@leoj3n
Copy link
Contributor Author

leoj3n commented Mar 19, 2022

See this issue #7017, there are links from there to the diff proposals.

@patak-dev What will take the place of suffixes? AFAICT the linked issue just swaps assert for as proposal but doesn't seem like that affects suffix API in any way?

@patak-dev
Copy link
Member

patak-dev commented Mar 19, 2022

We are going to get something like this in the future:

import foo from './file' as 'raw'

@leoj3n
Copy link
Contributor Author

leoj3n commented Mar 19, 2022

We are going to get something like this in the future:

import foo from './file' as 'raw'

@patak-dev Q is regarding suffix on URLs (http://localhost:3000/foo?raw), not the reflective import in files (see OP).

We will move away from suffixes

What this entails does not seem explained in #7017. Did you mean "move away from asserts"?

What will take the place of suffixes?

Maybe I'm not understanding but I don't get how using either reflective import or assert would affect the URL suffix.

@patak-dev
Copy link
Member

We aren't going to use the ?raw suffix anymore if we manage to use as 'raw', not only for glob imports but in general. Support for ?raw could be removed in a future major.

@leoj3n
Copy link
Contributor Author

leoj3n commented Mar 21, 2022

@patak-dev Ah, okay. If you care to, can you explain why ?raw was needed in the first place and why it is obviated by the move from assert to as? I assume I could figure it out if I were to study the code or wait for ?raw to be removed and see what changes then but am curious now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants