Skip to content

Ensure AST plugins have the same ordering as < ember-cli-htmlbars@5.5.0. - #665

Merged
rwjblue merged 1 commit into
masterfrom
properly-inverse-plugins-wtfbbq
Feb 27, 2021
Merged

Ensure AST plugins have the same ordering as < ember-cli-htmlbars@5.5.0.#665
rwjblue merged 1 commit into
masterfrom
properly-inverse-plugins-wtfbbq

Conversation

@rwjblue

@rwjblue rwjblue commented Feb 27, 2021

Copy link
Copy Markdown
Member

We have to reverse these for reasons that are a bit bonkers. The initial version of this system used registeredPlugin from ember-template-compiler.js to set up these plugins (because Ember ~ 1.13 only had registerPlugin, and there was no way to pass plugins directly to the call to compile/precompile). Calling registerPlugin unfortunately inverted the order of plugins (it essentially did PLUGINS = [plugin, ...PLUGINS]).

Sooooooo...... we are forced to maintain that absolutely bonkers ordering.

This should address #664, but I'd like to leave it open so @simonihmig can confirm.

We have to reverse these for reasons that are a bit bonkers. The initial
version of this system used `registeredPlugin` from
`ember-template-compiler.js` to set up these plugins (because Ember ~
1.13 only had `registerPlugin`, and there was no way to pass plugins
directly to the call to `compile`/`precompile`). Calling
`registerPlugin` unfortunately **inverted** the order of plugins (it
essentially did `PLUGINS = [plugin, ...PLUGINS]`).

Sooooooo...... we are forced to maintain that **absolutely bonkers**
ordering.
@rwjblue rwjblue added the bug label Feb 27, 2021
@rwjblue

rwjblue commented Feb 27, 2021

Copy link
Copy Markdown
Member Author

@rwjblue
rwjblue merged commit 6c0eaf6 into master Feb 27, 2021
@delete-merged-branch
delete-merged-branch Bot deleted the properly-inverse-plugins-wtfbbq branch February 27, 2021 04:19
@lukemelia

Copy link
Copy Markdown

Is there a list somewhere we can add this to so we make it rational in a breaking change for Ember 4.0? Or are stuck with this forever?

@rwjblue

rwjblue commented Mar 12, 2021

Copy link
Copy Markdown
Member Author

@lukemelia - Hmm. I would really like to find a way to do it, but I don't know of a reasonable path forward. We could easily guard this reversing code (and the code in Embroider that I linked) to avoid reversing when running against Ember >= 4, but then any addon that wants to support both Ember < 4 and 4 will have no way to actually guarantee the ordering.

@rwjblue

rwjblue commented Mar 12, 2021

Copy link
Copy Markdown
Member Author

Ironically, with Ember 4 the reason for reversing will actually be gone (since the registerPlugin based API is deprecated). 😭

@rwjblue

rwjblue commented Mar 12, 2021

Copy link
Copy Markdown
Member Author

@lukemelia - Mind making an issue to track the discussion / brain storming? Seems fine to do here, emberjs/rfcs, emberjs/ember.js, or Embroider repos...

@lukemelia

Copy link
Copy Markdown

@rwjblue created emberjs/rfcs#725

@rwjblue

rwjblue commented Mar 16, 2021

Copy link
Copy Markdown
Member Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants