Fixups for recent two pass babel changes.#6538
Conversation
When `this.options`, `this.options.babel`, or `this.options['ember-cli-babel']` have been clobbered, we do not handle the result gracefully. This adds some additonal warnings for various scenarios and adds fallback behavior to recover and avoid failures.
This patch will allow JS files to be used as templates in an addon. Without the `overwrite: true` there is a merge error as the merging at that point has a pre-compiled `.hbs` file in one tree and a post-compiled `.js` file in the other tree. If, however, you are relying on importing a template from another source and are using a `.js` file as the origin no template compilation takes place and the two trees have a similarly named file and a merge conflict results. It may be best to actually remove the original `.hbs` file from the original tree but that may add cost to the build step as those files are already being filtered out later. (cherry picked from commit ed99686)
|
seems good on first glance |
rwjblue
force-pushed
the
fixups-for-two-pass-babel-changes
branch
from
1820013 to
8614acf
Compare
Sadly, we cannot do "modules only" preprocessing due to changes in earlier versions of ember-cli where the second pass (that was intended for only modules transpilation) was actually implemented as a full babel transpile step. This brings back the prior behavior.
rwjblue
force-pushed
the
fixups-for-two-pass-babel-changes
branch
from
8614acf to
b281631
Compare
|
@bcardarella's fix is likely just fixing the symptom, not the problem. See: #5336. In favor of landing in spite of that. |
|
Yes, agreed there are more issues at play in #5336 for sure. I spent some time a while ago debugging that, and have a decent picture of what is wrong. I'll try to comment over there to give guidance. |
|
@homu r+ |
|
📌 Commit b281631 has been approved by |
…jblue Fixups for recent two pass babel changes. We started testing out master in a very large app at work, and ran into a few small issues with the changes in #6530. This PR makes the following changes: * Makes the warnings much more useful (and prevents an error that was occurring just when reading config to determine if we should warn). * Cherry pick change from #6480, as it is now manifesting (due to internal changes in `modules/` shimming, the `overwrite` that was happening at a later stage, is now happening in `compileAddon`). * Ensure a full pass of babel transpilation is done in the fallback case (when an addon with `addon/**/*.js` files has no JS preprocessors). This is required due to the way the migration from esperanto to babel occurred where the second pass (that was intended for only modules transpilation) was actually implemented as a full babel transpile step.
|
☀️ Test successful - status |
|
@rwjblue I lack some context, but I understand that some addons was (why?) deleting the babel configuration so the transpilation failed.
|
|
Before the changes in #6530 The RFC wouldn't have solved this specific scenario. |
|
@nathanhammond I agree, I think however that the correct path forward is a more comprehensive refactor of the entire build pipeline. We have file concat's happening throughout that maybe aren't ideal. I believe it would be best to have all files exposed in their original directory nested form in the same tree before their final phases of transpilation, funneling, then concat. This final "mega tree" should allow for a hook that can be used to make any compile-time changes to the state of that tree. Right now if you want to make compile-time optimizations it is impossible to get a comprehensive view of all project files (vendor and app). |
6 participants
We started testing out master in a very large app at work, and ran into a few small issues with the changes in #6530.
This PR makes the following changes:
modules/shimming, theoverwritethat was happening at a later stage, is now happening incompileAddon).addon/**/*.jsfiles has no JS preprocessors). This is required due to the way the migration from esperanto to babel occurred where the second pass (that was intended for only modules transpilation) was actually implemented as a full babel transpile step.