Skip to content

feat: compose addon hooks#2007

Merged
IWANABETHATGUY merged 1 commit intorolldown:mainfrom
ikkz:feat/compose-hooks
Aug 18, 2024
Merged

feat: compose addon hooks#2007
IWANABETHATGUY merged 1 commit intorolldown:mainfrom
ikkz:feat/compose-hooks

Conversation

@ikkz
Copy link
Copy Markdown
Contributor

@ikkz ikkz commented Aug 17, 2024

resolves #1999

@netlify
Copy link
Copy Markdown

netlify bot commented Aug 17, 2024 •
edited
Loading

✅ Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 1af96d9
🔍 Latest deploy log https://app.netlify.com/sites/rolldown-rs/deploys/66c1a1f6017fe90009fd9487

@IWANABETHATGUY
Copy link
Copy Markdown
Member

IWANABETHATGUY commented Aug 17, 2024

Can you double-check the implementation? It did not compose at all.

@ikkz
Copy link
Copy Markdown
Contributor Author

ikkz commented Aug 17, 2024

Can you double-check the implementation? It did not compose at all.

I added some log here

image

and output of just test-node rolldown -t "multi-addon-composing":

image

It seems that the 8 test plugins have been composed to 1 plugin.

Did I misunderstand this issue?

@IWANABETHATGUY
Copy link
Copy Markdown
Member

IWANABETHATGUY commented Aug 17, 2024

plugins

It's my mistake, we need a technique to determine if the plugin is composed, I am working on that.

@IWANABETHATGUY
Copy link
Copy Markdown
Member

IWANABETHATGUY commented Aug 17, 2024

Can you add a test like https://github.com/rolldown/rolldown/pull/2008/files#diff-e709cc275fbba560e8cd300f426f59152876a8ce9c02bf8283a61fdc258cf319, so we could verify it is correctly composed

@ikkz ikkz force-pushed the feat/compose-hooks branch from 88f5eb8 to 1af96d9 Compare August 18, 2024 07:25
@ikkz
Copy link
Copy Markdown
Contributor Author

ikkz commented Aug 18, 2024

Can you add a test like https://github.com/rolldown/rolldown/pull/2008/files#diff-e709cc275fbba560e8cd300f426f59152876a8ce9c02bf8283a61fdc258cf319, so we could verify it is correctly composed

Done

IWANABETHATGUY
IWANABETHATGUY approved these changes Aug 18, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@IWANABETHATGUY IWANABETHATGUY left a comment

Choose a reason for hiding this comment

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

thanks

@IWANABETHATGUY IWANABETHATGUY added this pull request to the merge queue Aug 18, 2024
Merged via the queue into rolldown:main with commit 08b6360 Aug 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@IWANABETHATGUY IWANABETHATGUY IWANABETHATGUY approved these changes

Assignees

No one assigned

Labels

needs-triage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Feature Request]: Support banner, footer, intro and outro for composejs plugin

2 participants

@ikkz @IWANABETHATGUY