Disable Ember modules polyfill on 3.27+#337
Merged
RobbieTheWagner merged 1 commit intohtml-next:ember-3.16from Oct 6, 2021
Merged
Disable Ember modules polyfill on 3.27+#337RobbieTheWagner merged 1 commit intohtml-next:ember-3.16from
RobbieTheWagner merged 1 commit intohtml-next:ember-3.16from
Conversation
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.
Contributor
Author
rwjblue
approved these changes
Oct 6, 2021
|
Yes, this rollup usage is fragile and of dubious value. Even if it's beneficial, it should run at a prepublish and not within every app's build. |
Member
|
I would be okay with removing the custom private rollup stuff if @runspired is. Thoughts @runspired? |
mixonic
pushed a commit
that referenced
this pull request
Dec 8, 2021
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.
mixonic
pushed a commit
that referenced
this pull request
Dec 8, 2021
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.
mixonic
pushed a commit
that referenced
this pull request
Dec 9, 2021
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.
mixonic
pushed a commit
that referenced
this pull request
Dec 9, 2021
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.
mixonic
pushed a commit
that referenced
this pull request
Dec 9, 2021
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.
mixonic
pushed a commit
that referenced
this pull request
Dec 9, 2021
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.
mixonic
pushed a commit
that referenced
this pull request
Dec 9, 2021
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.
mixonic
pushed a commit
that referenced
this pull request
Dec 9, 2021
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.
mixonic
pushed a commit
that referenced
this pull request
Dec 9, 2021
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.
mixonic
pushed a commit
that referenced
this pull request
Dec 9, 2021
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.
mixonic
pushed a commit
that referenced
this pull request
Dec 9, 2021
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.
mixonic
pushed a commit
that referenced
this pull request
Dec 9, 2021
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When compiling with
compileModules: false, ember-cli-babel defaults to using the modules polyfill, since it assumes we are concatenating the output script usingapp.importwithout an AMD wrapper.This does not apply to us, since we are compiling the
-privatemodules 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
trueunconditionally, andember-cli-babelwill 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.https://github.com/babel/ember-cli-babel/blob/4c3b9091d7c711ecb804a52226409b409a702d82/lib/babel-options-util.js#L159-L172
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.