Skip to content

refactor: use macros to reduce duplicated codes in injection fields.#1755

Closed
7086cmd wants to merge 1 commit intorolldown:mainfrom
7086cmd:refactor/uniform-banner-etc
Closed

refactor: use macros to reduce duplicated codes in injection fields.#1755
7086cmd wants to merge 1 commit intorolldown:mainfrom
7086cmd:refactor/uniform-banner-etc

Conversation

@7086cmd
Copy link
Copy Markdown
Contributor

@7086cmd 7086cmd commented Jul 27, 2024

Introduction

This PR introduces several improvements and new features, focusing on code optimization and support for additional plugin functionalities.

Contents

  1. Code Optimization using Macros:

    • Introduced macros to reduce duplicated code in ecma_generator.rs, specifically targeting the format renderers.
    • The Plugin trait is excluded from macro optimization due to an issue with async traits not being compatible with the macros.
  2. Plugin Enhancements:

By utilizing macros, we've significantly reduced the amount of duplicated code, making the codebase cleaner and easier to maintain.

@netlify
Copy link
Copy Markdown

netlify bot commented Jul 27, 2024

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 315c07e
🔍 Latest deploy log https://app.netlify.com/sites/rolldown-rs/deploys/66a4c84c1beb9a00081116b3

@hyf0
Copy link
Copy Markdown
Member

hyf0 commented Jul 27, 2024

Using macros is rejected. It's bad for maintaining.

@7086cmd 7086cmd closed this Jul 27, 2024
@Boshen
Copy link
Copy Markdown
Member

Boshen commented Jul 27, 2024

We try to avoid macros as much as we can. A macro should be used for DSLs or super repetitive work.

The usage here can be replicated by simple functions (I think).

@hyf0
Copy link
Copy Markdown
Member

hyf0 commented Jul 27, 2024

Yeah. Duplication isn't bad always. Introducing abstraction too early might be bad for future refactoring.

@7086cmd
Copy link
Copy Markdown
Contributor Author

7086cmd commented Jul 27, 2024

I see. But the logic of these four is simply the same. I am wondering if there is a better way except for macros instead of copying them...?

github-merge-queue bot pushed a commit that referenced this pull request Jul 28, 2024
#### Contents

1. **Code Optimization:**
   - Refined types like `HookBannerArgs` to `HookInjectionArgs`, etc.
   - **macros in #1755 are removed now**.

2. **Plugin Enhancements:**
- Added support for `intro` and `outro` in plugins, using #1713 as a
reference example.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants