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!: allow to run Babel on non js/ts extensions (fixes #38) #53

Closed
wants to merge 3 commits into from

Conversation

ArnaudBarre
Copy link
Member

@ArnaudBarre ArnaudBarre commented Dec 8, 2022

This PR is currently composed of 3 commits:

  • First, just skip everything if id include node_modules. Given that esbuild pre-bundle deps, it seems strange for me that you need to process them again via Babel. I'm not sure we should encourage people to rely on the fact that they use node_modules with non standard content
  • Second, main commit: use the include/exclude like other plugins, with a default include that was the previous mandatory check. I don't see why you would want to skip fast refresh if it can be enable. This simplifies a lot the code and the configuration options compare to feat(plugin-react): change how babel.include/exclude options behave vite#6202. Finally I think that changing the behaviour when the import is outside vite root but still imported as source is confusing. I have bad memories of CRA disabling simple code sharing between two projects and requiring to setup a fake local lib.
  • Third, add an mdx playground with a simple HMR test

Fixes #38, fixes #22, fixes #24 and should also fix #16

@ArnaudBarre ArnaudBarre self-assigned this Dec 8, 2022
@ArnaudBarre ArnaudBarre changed the title feat!: allow to run Babel on non js/ts extensions (fix #38) feat!: allow to run Babel on non js/ts extensions (fixes #38) Dec 8, 2022
@patak-dev
Copy link
Member

I think we should wait for this one a bit, given that the release of Vite 4 is so close. Now that the plugin is is in a separate repository we can do a new major for it independent of Vite after the holidays. We can do the a proper beta period and check with the ecosystem.

Comment on lines -214 to +219
if (/\.(?:mjs|[tj]sx?)$/.test(extension)) {
if (filter(id)) {
Copy link
Member

Choose a reason for hiding this comment

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

I think this changes behavior when id is foo.txt?a=.jsx. (it was true but now it's false)

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