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

Introduce runPluginFirst option #254

Closed
wants to merge 1 commit into from
Closed

Introduce runPluginFirst option #254

wants to merge 1 commit into from

Conversation

guyca
Copy link

@guyca guyca commented Dec 14, 2024

If set to true WASM plugins will be executed before TS transformations. Enabling this option is required for plugins that mutate decorators as they rely on the standard TS syntax and can't mutate transformed decorators. The existing behavior of running plugins after TS transformers is still the default behavior.

Usage

import { defineConfig } from 'vite'
import react from "@vitejs/plugin-react-swc";
import obsidian from 'swc-plugin-obsidian'

export default defineConfig({
  plugins: [
    react({
      runPluginFirst: true, // the obsidian plugin will be executed before TS transformations
      tsDecorators: true,
      plugins: [ obsidian() ]
    })
  ]
})

If set to true WASM plugins will be executed before TS transformations. Enabling this option is required for plugins that mutate decorators as they rely on the standard TS syntax and can't mutate transformed decorators.
The existing behavior of running plugins after TS transformers is still the default behavior.
@guyca
Copy link
Author

guyca commented Dec 15, 2024

@ArnaudBarre Would you like me to update the readme as well?
This PR only exposes a config option of @swc/core and I used the same naming. I'm not sure the name is runPluginFirst explains clearly what the option does. Would you like to use a different name? IMO runPluginsBeforeTypeScriptTransformations is clearer 😅

@ArnaudBarre
Copy link
Member

Hey @guyca,
For reasons explained in other issues/PRs, recently here, we will not add to the API options that facilitates the use of legacy decorators. A patch is still an available option if you want to continue with this approach, but we would not make it part of the API.

@guyca
Copy link
Author

guyca commented Dec 15, 2024

Hey @ArnaudBarre, thanks for your reply!
This is actually not related to decorators version - the runPluginFirst in @swc/core is required also when using the latest Stage 3 proposal.

The issue is that when TypeScript transforms decorators, regardless of version, it actually strips the decorators and converts them a different syntax. Thats why plugins that want to transform decorators requires the runPluginFirst. Because they need to be executed by SWC before TypeScript transforms the decorators.

Once again - this PR is 100% agnostic to decorators version and will be required by all future versions and iterations of the decorators proposal.

@ArnaudBarre
Copy link
Member

I'm very septical of this SWC api anyway: having either all plugins run before or after the TS transformation (and code lowering) is will not scale well. On vite the capability for plugins to be resister themself as "pre" or "post' is quite an important feature to give people control on their code transformation.

I'm also septical about the need of running code transformation on decorators. What is your need for that?

@guyca
Copy link
Author

guyca commented Dec 16, 2024

I'm the author of wix-incubator/obsidian - a dependency injection (DI) framework for React and React Native applications. DI frameworks commonly use "injection tokens" when injecting dependencies. For example:

class Foo {
  @inject(Tokens.BAR) bar: Bar; // inject a bar instance
}

// IoC container that maps tokens to dependencies
const container = new Container()
container.bind(Tokens.Bar).to(Bar)

Concrete examples of this can be found in the InversifyJS docs or in microsoft/tsyringe both are very popular DI frameworks.

Use case 1

Personally I find this too verbose so in Obsidian we have Babel and WASM plugins that add the tokens automatically at transformation time.

Before transformation After transformation
class Foo {

  // No need to explicitly pass an injection token -
  // it's created automatically at transformation time
  @inject() bar: Bar;
}
class Foo {

  // the WASM plugin sets `bar` as the token
  @inject('bar') bar: Bar;
}

Use case 2

Like all other DI frameworks that are used to follow that Inversion of Control principle, we also have IoC containers. We use Babel and WASM to transform them to improve performance. We use a transformer to slightly alter methods decorated with a @provides() decorator so their arguments are resolved lazily at runtime.

Before transformation After transformation
@singleton() @graph()             
class FrameworkDependencies {
  @provides()
  foo(bar: Bar, baz: Baz) {
    return new Foo(bar, baz);
  }
}
@singleton() @graph()                                
class FrameworkDependencies {
  @provides({name: 'foo'})
  foo({bar, baz}) { // At runtime, Obsidian passes a Proxy object that resolves the required dependencies lazily
    return new Foo(bar, baz);
  }
}

Currently I have to direct users to use unplugin-swc which supports passing the runPluginFirst option.

I appreciate the discussion and the time you've taken to consider this feature. Thanks again for maintaining such an excellent project and for being so attentive and responsive 😇

PS, the current version of Obsidian still uses legacy decorators, but in v3 which is in alpha we transitioned to the latest implementation of decorators.

@guyca
Copy link
Author

guyca commented Dec 19, 2024

Hey @ArnaudBarre
Hope you're well, sorry to bother you again. Did you get a chance to read my previous message with the explanations and motivation?

I would like to re-emphasize three things:

  1. This PR only exposes a configuration of @swc/core - since vite-plugin-react-swc is essentially a wrapper over @swc/core some configurations need to be exposed.
  2. The runPluginFirst options is required by some plugins
  3. This PR is agnostic to decorators version. It has nothing to do with the legacy version of decorators. The runPluginFirst is required even when transforming the latest decorators implementation.

Have a nice weekend,
Guyca :)

@ArnaudBarre
Copy link
Member

The philosophy of the plugin is more to be a faster version of the default one, and while SWC is not entirely a implementation detail, it not something we want people to be too much locked in. Once Vite ships with OXC, for me there are no benefits of downloading a 30MB rust toolchain to do what OXC does already and so I'm not planning to maintain this long after.
I asked the maintainers of OXC on their plan to implement this flag, if they plan to add it I'll expose the flag.

Otherwise I think that what I could add is an useAtYourOwnRiskEditConfig that allow to mutate the config and add any flag you want, while documenting the fact that your are in non officially supported area

@guyca
Copy link
Author

guyca commented Dec 19, 2024

Thank you very much for your thoughtful response and the consideration you've put into this matter. As a fellow maintainer of OSS projects - I truly appreciate it.

Both options seem good to me. Obviously I'd be happier with option 1 but option 2 works just as well.
Once again, thanks a lot!

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.

2 participants