-
-
Notifications
You must be signed in to change notification settings - Fork 408
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
Ember builds without conflicts of globals #781
Comments
This is interesting! I have a question (which is probably pretty naive) -- why does Ember need to continue to use an AMD-style loader? Why doesn't Ember move to export ES modules by default and provide the existing AMD-style loader as an optional build target? Is this because of browser support issues or limitations in the framework or something else? |
I really want ESM as a target format. It would help all my projects immensely (and enable skypack usage), and give a pretty good build-speed boost, as there would be no conversion to AMD (at least for dev-time). ESM for production builds would be technically challenging, cause you'd need to still create a concattenated file of uniquely named things, and you lose module boundaries... which could be dangerous in some cases (side-effects, name collisions, etc) for REPLs especially, I really want to be able to do this: |
@simonihmig I'm very much in support of something along these lines, and I'd like to contribute another use case, which is using To work around this,
It works, but it's not pretty, clearly has cases where it would fail (Ember-bundled modules with the same name as Node.js modules but different functionality), and I would love it if something like this RFC removed the need for this hackery. |
One more case to highlight here - we have to figure out how to share namespace visibility between engines (it's considered as a "standalone" apps, but should be aware of parent app and it's modules) |
Oh, that's an excellent addition! I didn't know about that, will add it when preparing the real RFC! Thanks @bendemboski
Oh, yes, good point! (not an engines user, so I tend to forget about them). Thanks @lifeart |
Actually, this is a very good question! I am by far not the best to answer that, but I'll try by saying it is probably a combination of...
|
@bendemboski I was iterating a bit on my RFC draft, adding your use case, when I realized that the proposal I had in mind (wrapping code in an IIFE to prevent polluting the global namespace, while still allowing to use I think the only way to address this is to actually get totally rid of |
@simonihmig I may need to clarify something. The
I'm pretty sure (2) would be unaffected by the IIFE setup, right? Because code loaded via Node.js' module loader would be unaffected by the IIFE, and references to the global So as far as So that leaves (1), which is a stickier problem because there is inherent ambiguity -- if bundled source code says So I think this RFC does help However, I think there's another consideration. If this RFC were implemented today with no changes to |
@bendemboski thanks a lot for your extensive feedback!
I was indeed worried about your case 2) here, but you are right, the IIFE wouldn't change anything. I didn't think this through correctly. So yeah, that's good actually!
Yes, I agree that would be the cleaner solution than the overloaded
Yes. But that's why we wrap that new behavior in an optional feature, as there are edge cases like this one where this is a breaking change. So with the RFC implemented, the message for electron users would be that it is recommended (and the old setup not using |
Closing this in favor of the real thing: #784. Thanks for all your feedback so far! |
@simonihmig yes, I agree. Assuming nothing is changed that breaks |
This is a draft of an RFC, for which I'd like to get some early feedback. Will then finalize and submit it as a real RFC.
Summary
Due to the use of the global namespace, especially for its AMD-style module system, Ember apps have problems coexisting
with other Ember apps or any AMD-based JavaScript on the same page. This RFC proposes a way to prevent all potentially
conflicting usage of these globals.
Motivation
The most common case for running an Ember app is to run it as a pure isolated SPA (Single Page Application), i.e. to have
it own the HTML page it runs on. This is what you get out of the box when running
ember build
: a (mostly empty)index.html, that contains the build assets that run the Ember app inside that empty page.
But another use case is to embed it into an existing page,
so that it runs alongside other JavaScript (let's call it "3rd party" from the perspective of the Ember app). While that
might work fine in some cases, due to Ember's still present reliance on globals, especially its AMD-style module
system that is actually incompatible with the original one of RequireJS, there is a high chance of non-trivial conflicts.
Let's look at a few examples:
Embedding an Ember app into a 3rd party HTML page
You might want to embed an Ember app on a 3rd party page (i.e. one that you don't control), for example
In these cases, if the 3rd party page uses RequireJS itself, only one implementation (Ember vs. RequireJS) of the globally
defined
define()
andrequire()
functions will "win", and the other's module loading will inherently be broken, inthe worst case both.
This is not uncommon, for example Magento, the most wide-spread e-commerce solution worldwide,
uses RequireJS, so effectively
you cannot use an Ember app on its pages without somehow tackling that conflict.
Micro Frontends
A rather new pattern in frontend development is the analogy of microservices for the frontend world, coined
Micro Frontends, where you develop, test and deploy parts of
your frontend independently, and compose them together for a seamless user experience.
The same issue arises when one of those micro frontends happens to use RequireJS, or two or more frontends are actual
Ember apps. While in the latter case we don't have to deal with different implementations of
define()
andrequire()
,those functions close over their module registry, so whatever functions win (as being the globally defined one), they will
only be able to see their registered modules, the modules of the other app will not exist anymore. And we are not even
speaking about conflicting versions of the same modules (say
@ember/component
). So in any way, that scenario is boundto fail miserably.
Detailed design
The guiding principle suggested in this RFC is to make Ember apps practically side effect free, with respect to the
usage of the browser's global namespace (
window
/globalThis
).That means it should be discouraged to
While this can only be a recommended but not enforceable best practice for user-land code, for Ember and its build
tooling this should become a rule. That specifically includes:
Where this is practically not possible, it should be ensured that there is no realistic chance of a conflict, by
package.json
), so that two different Emberapps cannot cause conflicts
privileged code (as listed above), so user-land code cannot rely on the existence of these globals.
While the
Ember
global has already been deprecated and removed from Ember 4 as part of RFC 706,there are still other variables that Ember apps leak into the global namespace as described below, most notably Ember's
AMD-like module system, and as such pose a risk for conflicts.
Opt-in behavior
The changes proposed here should ideally only require changes in Ember's build tooling. For idiomatic Ember code, especially
code that only uses ES modules, it should be a non-breaking change. Also code that uses Ember's AMD-style module system
directly like
require()
orrequire.entries
, should continue to work without any changes in most cases, as long as thecode is built by Ember-CLI (see below).
But there might be cases where people are doing things differently, and code - even after the build tooling changes
proposed below - still effectively relies on the globally defined AMD-like module API. For this reason we may not just
remove these from the global namespace, breaking backwards compatibility.
So this RFC proposes an opt-in behavior, by introducing a new "feature" based on the already well known
@ember/optional-features
, namelyno-globals
.All the following proposals only apply when a user as explicitly opted into the new behavior, by enabling that optional
feature!
Remove globals usage
As described above, when opting into
no-globals
mode, an Ember app built using Ember-CLI, including optionallyember-auto-import and/or Embroider, should not read from and leak any globals, with the exceptions outlined above.
The following identifies current global usage, and outlines possible ways to remove their usage, or at least prevent the
chance of conflicts. Note that this RFC does not aim to prescribe a specific implementation, as besides the visible
behavior of not relying on globals there is no public API introduced here, so the actual implementation remains the task
of any implementor of this RFC. The implementation outlines are merely intended to demonstrate the feasibility of the
proposed changes.
AMD-like module system
The global functions/namespaces
define
andrequire
(and its aliasesrequireModule
,require
,requirejs
) are setas globals and provided by the
loader.js
addon, which is part of the defaultblueprint of any Ember app. Not having this addon or any equivalent replacement will break the app, as a.o. any
define()
calls in
vendor.js
(for addon code) orapp.js
(for app code) would call an undefined function. So it is effectively amandatory dependency currently.
Despite that, Ember comes with its own (stripped down) loader library,
that has been used (pre Ember 3.27) internally only. As of Ember 3.27 it is effectively unused when
loader.js
is present(which it has to be, see above), so dead code effectively.
With that, a typical build of
vendor.js
will look basically like this:vendor.js
And this is what a typical compilation of
app.js
will look like:app.js
As you can see it relies on a globally defined
define
, that has been declared in the first line ofvendor.js
, and setby the code of
loader.js
.As to how to prevent the global usage while retaining compatibility of any code that uses those functions as globals,
we can make the build system emit the existing code wrapped in a IIFE,
that avoids polluting the global namespace, while still any calls of
define()
orrequire()
will see the same functions,they just do not come from the global namespace anymore.
This works for
vendor.js
, where these functions are defined, but not forapp.js
which needs access to the same functionsas defined in
vendor.js
. That's why we still need some usage of globals, but as outlined in the previous chapter, wemust make sure they are scoped to the current app, so they pose no risk of conflicts.
So for
vendor.js
this is what the basic change could look like:vendor.js
And this for
app.js
:app.js
By wrapping the code in the IIFE and defining
define
andrequire
within it, we are basically hiding any eventually definedglobals (e.g. 3rd party RequireJS), while letting all existing code use our implementation as if it were globally defined.
Besides these two bundles, which make up "classic" Ember apps, we must also take into account the additional bundles
("chunks") that ember-auto-import and Embroider would emit.
ember-auto-import emits a
chunk.app.xxx.js
chunk, which includes the boundary between Ember's AMD-like modules andwhatever Webpack emits for the imported modules it handled, so this is basically the glue between these two worlds. It typically
looks something like this (cleaned up and simplified):
chunk.app.xxx.js
Note that ember-auto-import itself creates new globals
_eai_*
here, that we must prevent. And it takes Ember'srequire
and
define
from the global namespace. A "fixed" version could look like this:chunk.app.xxx.js
EmberENV
Ember reads environment variables from a
EmberENV
hash to enable/disable certain runtime features. Usually this is setfrom the app's
config/environment.js
and@ember/optional-features
, and compiled into the top ofvendor.js
:vendor.js
But as you can see there, you can also inject those by setting a globally defined
window.EmberENV
.In
no-globals
mode though, the latter behavior will be disabled. Only the variables built intovendor.js
are takeninto account. Implementation wise this can happen in the same way as described above, by defining
EmberENV
within theIIFE (and not using the explicit
window.
namespace).Deprecate direct usage of AMD-like module APIs
TO DO
When opting into
no-globals
, it seems reasonable to also deprecate direct usage ofdefine
andrequire
(the latter is not uncommon)in user-land code. We can still make the code emitted by the build system not cause any deprecations, but add deprecations
at least for
require
in user's code. I hope for any need of dynamic module usage, we can provide a transition pathusing
embroider/macros
(using e.g.dependencySatisfies()
andimportSync()
). This would make the AMD module systembecome a private implementation detail, thus easing possible future changes of the way we package apps!?
Should this be part of this RFC? It could be a separate, on the other hand it is tied to the new
no-globals
mode proposed here.How we teach this
TO DO
Drawbacks
TO DO
Alternatives
TO DO
Unresolved questions
noConflict()
,.unsee()
,.alias
, what else? Remember that we are opting into a new mode, so we can drop things and define our new standard, that is basically a strictly private API now, right?Ember.__loader
? Still needed? How is it even supposed to be used, with the Ember global gone? If you are supposed torequire('ember')
, you already used the globalrequire
, so what isEmber.__loader.require
for?The text was updated successfully, but these errors were encountered: