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

feat: respect resolved id instead of raw request id #58

Merged
merged 1 commit into from
Mar 24, 2022
Merged

feat: respect resolved id instead of raw request id #58

merged 1 commit into from
Mar 24, 2022

Conversation

lxy-yz
Copy link
Contributor

@lxy-yz lxy-yz commented Mar 23, 2022

I could be missing something, it seems more reasonable to use resolved id so that plugin author have more control over resolveId hook.

Problem

Have a use case for virtual routes as below:

// App.tsx
import routes from '~routes'

// webpack.config.js
// `~routes` w/o extension will be matched by CRA's file-loader 
// https://github.com/facebook/create-react-app/blob/eee8491d57d67dd76f0806a7512eaba2ce9c36f0/packages/react-scripts/config/webpack.config.js#L509


// Plugin.ts
{
  resolveId(id: string) {
    if (id === '~routes') 
      return '~routes.mjs'
    return null 
  }
}

@antfu
Copy link
Member

antfu commented Mar 23, 2022

The tests are failing. IIRC there was a reason to not use the resolved one but I am not sure.

@lxy-yz
Copy link
Contributor Author

lxy-yz commented Mar 23, 2022

The tests are failing. IIRC there was a reason to not use the resolved one but I am not sure.

Will attempt to fix the failing test first and see :)

@antfu antfu changed the title chore: respect resolved id instead of raw request id feat: respect resolved id instead of raw request id Mar 24, 2022
@antfu antfu merged commit 468da3e into unjs:main Mar 24, 2022
@antfu
Copy link
Member

antfu commented Mar 24, 2022

Ok, let's see if it works for the downstream

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