-
Notifications
You must be signed in to change notification settings - Fork 76
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
Update CI to Ember 2.18+ #343
Conversation
7f1a139
to
a184b64
Compare
Cool, this is basically proven out. With no real changes other than bumping |
ec90107
to
c119d28
Compare
Alright! I've gone through a number of failures and looked at #335 in detail as well. This is getting close though I've still got a failure. This evening I backported four patches which can safely land on
That is all released in v2.1.0. The content of what remains in this branch/patch is:
|
8541c61
to
2fd5744
Compare
3b4f9cf
to
bb22f44
Compare
5f47f15
to
2e29799
Compare
When compiling with `compileModules: false`, ember-cli-babel defaults to using the modules polyfill, since it assumes we are concatenating the output script using `app.import` without an AMD wrapper. This does not apply to us, since we are compiling the `-private` modules into a single AMD module (via rollup below), which can in fact have external dependencies. We can opt-out of this with `disableEmberModulesAPIPolyfill: true`. In Ember versions with "real modules", that is what we want in order to avoid the Ember global deprecation (or just completely not working in 4.0+). It seems like the intent may have been that we should be able to set this to `true` unconditionally, and `ember-cli-babel` will ignore this setting if the Ember verion requires the modules API polyfill. However, presumably due to a bug, ember-cli-babel actually checks for this value first and return out of the function early if its value is truthy. This means that if we set this to true unconditionally, then we would have disabled the modules polyfill for Ember versions that needs it, which would be incorrect. Therefore, we have to duplicate the detection logic here in order to set this value appropriately. Ideally, we should just stop trying to rollup the -private modules and let the modern build pipeline optimizes things for us, then none of this would have been necessary.
Ok, green and landing it. I'll cut Thanks @rwwagner90 @chancancode @johanrd for the illustrative work on #335. That was a powerful guide. I think most of the critical components from that PR are represented here, the biggest difference in lines being that this PR doesn't attempt to change the linting and code style. I'd love to work with you on getting any of that landed as incremental changes on the main branch. |
|
A patch to start v3 on
master
. Will support Ember 2.18.0 through latest, will support Node 14+.If this lands, we should also start a
release-2
branch for patches back to 2.x.