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(plugin-legacy): Add experimental option to use swc. #4105

Closed
wants to merge 1 commit into from
Closed

feat(plugin-legacy): Add experimental option to use swc. #4105

wants to merge 1 commit into from

Conversation

avocadowastaken
Copy link
Contributor

@avocadowastaken avocadowastaken commented Jul 3, 2021

Description

DISCLAIMER: It's first time I'm using SWC, so a lot of things here are not final.

  • This PR is just the proof of concept for the further consideration of the swc usage
  • I tried to make minimal changes, so it would be easier to check changes and compare files side by side
  • I've considered to make a separate plugin first, but there are a lot of things going on in original plugin, and the actual transformation part that has to be updated is small. (also original plugin is uses internal flag vite_skip_esbuild, which I assume can be changed or removed in future)

Additional context

Cons:

  • I didn't use official Visitor class from @swc/core/visitor, since it was breaking output code in weird ways, and I din't want to waste time on investigation
  • I couldn't make source maps work, looks like passing source maps generated from one swc transformation to another as inputSourceMap does not work
  • Overall I got a feeling that swc is good enough if you're using it's built in transformers and not messing with the plugin api.
    • no way to run plugins after main transformations (that's why I had to run transforms twice)
    • Visitor api is very limited (not to mention is buggy as I've pointed before) and better to be avoided (missing super call can break whole traverse chain)
    • no helpers to create ast objects, everything has to be created manually, and I had to put dummy spans everywhere

Pros:

Maybe changing direction, and instead of transforming the final chunk it would be better to run swc in transformation hook, and get rid of custom plugins. But it will require a lot of internal changes, and better to be extracted as an external plugin.


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.

@Shinigami92 Shinigami92 added enhancement New feature or request plugin: legacy labels Jul 4, 2021
@patak-dev
Copy link
Member

Thanks for the PR @umidbekk, this is an interesting alternative! We'll discuss this with the team. My personal take is that we could first try to have this approach as a separate vite-plugin-legacy-swc plugin, and we may end up merging this upstream at one point. But let's wait to see what others think.

@patak-dev
Copy link
Member

Hello @umidbekk, we discussed this proposal with the team and we think this approach is worth exploring. The only issue apart from polishing this solution is that for the moment we should keep both babel and swc (in case people want to keep using the babel based alternative due to maturity and stability), and introducing this dependency could be heavy for people not using it.
A good way forward is that you could offer a new vite-plugin-legacy-swc that is a fork of the official plugin respecting and following its API but based on swc. Once this plugin is mature enough, we can link to it from the official plugin-legacy docs and encourage people to use it. At one point in the future, we could directly made the switch in the official vitejs org plugin-legacy.
What do you think?

@avocadowastaken
Copy link
Contributor Author

Hello @umidbekk, we discussed this proposal with the team and we think this approach is worth exploring. The only issue apart from polishing this solution is that for the moment we should keep both babel and swc (in case people want to keep using the babel based alternative due to maturity and stability), and introducing this dependency could be heavy for people not using it.
A good way forward is that you could offer a new vite-plugin-legacy-swc that is a fork of the official plugin respecting and following its API but based on swc. Once this plugin is mature enough, we can link to it from the official plugin-legacy docs and encourage people to use it. At one point in the future, we could directly made the switch in the official vitejs org plugin-legacy.
What do you think?

Hi @patak-js thank for the review!

I also hate the idea of installing optional packages (especially with giant binaries), and I agree that using separate package is the best way to approach.

One think that stops me now is vite_skip_esbuild flag. But I have an idea how to handle that - add use esbuild plugin that will transform *-legacy.* chunks itself, so esbuild would not modernize them by mistake.

But there another problem I see with plugin-legacy - it has two purposes:

  1. Emit legacy bundle
  2. Scan source and create polyfill bundle

Maybe using separate plugin to generate polyfill bundle was the better approach. But again, vite was made for the modern browsers, and maintenance of this plugin should not be a core team priority.

Also, I don't even use polyfill bundle myself, and use polyfill.io instead, I only thinking about it so vite-plugin-legacy-swc would be fully backward compatible with the @vitejs/plugin-legacy.

I guess it's better to start with the minimal working approach, and start adding things on demand.

I will close this, and start working on the esbuild plugin proposal for the plugin-legacy.

@avocadowastaken avocadowastaken deleted the legacy-swc branch July 11, 2021 10:46
@patak-dev
Copy link
Member

Also, I don't even use polyfill bundle myself, and use polyfill.io instead, I only thinking about it so vite-plugin-legacy-swc would be fully backward compatible with the @vitejs/plugin-legacy.

I guess it's better to start with the minimal working approach, and start adding things on demand.

Sounds good. Since it will be your plugin, I think you should design it as you see fit. The idea of keeping the API as close as possible was to enable an easy transition in the future if we go that road. But it would be a new mayor version and the API can be changed if there are good reasons.

I will close this, and start working on the esbuild plugin proposal for the plugin-legacy.

Thanks again and please share your work later on https://chat.vitejs.dev!

@Shinigami92
Copy link
Member

Should we somehow promote it especially on https://github.com/vitejs/awesome-vite?

@patak-dev
Copy link
Member

Should we somehow promote it especially on vitejs/awesome-vite?

Let's wait to see how the plugin evolves and is being used, for sure it should be listed there. I think there isn't any concept of that in awesome-vite, to me a link from the readme of plugin-legacy is good here (and maybe a note in the production section of the docs).

@CyanSalt
Copy link

CyanSalt commented Feb 9, 2023

As this PR is now closed, I have ported it with the latest plugin-legacy and released the package vite-plugin-legacy-swc. If anyone else has the same needs as me, feel free to try it out and make suggestions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request plugin: legacy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants