Open bobisjan opened 5 years ago
I'm definitely in favor of doing things to simplify and cleanup how to work with FastBoot, and this does seem like a logical thing to reevaluate. There are a few things that I don't fully understand how you would do in the new system:
require
(defined here), I don't see how you'd gain access to the Node-land require
without having it exposed in some way (e.g. FastBoot.require
).FastBoot.require
today allow for async absorption)ember-auto-import
(and embroider
) aware of this specific pattern, so that it knows that the shim it uses to implement dynamic requires should use node-land vs in-app systems?If we do move forward with this proposal, I think that we would need a decent length deprecation cycle for FastBoot.require
. So this probably wouldn't be something we can straight up remove in v3.0.0 (which gives us a bit of time to review/analyze the best path forward).
Since inside an Ember application there is already a global variable named require (defined here), I don't see how you'd gain access to the Node-land require without having it exposed in some way (e.g. FastBoot.require).
If I understand ember-auto-import
correctly, the FastBoot.require
is not used for importing from 3rd packages in FastBoot, because add-on handles this on its own via webpack.
The problem is with Node.js built-in modules, for that the FastBoot.require
should be still required if using buildSandboxGlobals()
is not ideal. Another option might be to allow dynamic imports of Node.js builtins in FastBoot - ember-auto-import
will probably still be using internally FastBoot.require
, but that would be abstracted away from end-user.
How do we absorb the asynchrony introduced? (not all locations that you would need to FastBoot.require today allow for async absorption)
You mean that await import('...')
at top level is not supported today in Ember? Maybe some temporary dynamic but static importSync()
provided by ember-auto-import
can be introduced to proceed before top level await is supported (this will not use FastBoot.require
)?
Do we need to make ember-auto-import (and embroider) aware of this specific pattern, so that it knows that the shim it uses to implement dynamic requires should use node-land vs in-app systems?
I'm not sure if I understand correctly, but the ember-auto-import
seems to already know how to dynamically import 3rd packages both when it is running in browser or in FastBoot.
If we do move forward with this proposal, I think that we would need a decent length deprecation cycle for FastBoot.require. So this probably wouldn't be something we can straight up remove in v3.0.0 (which gives us a bit of time to review/analyze the best path forward).
That makes totally sense 👍.
If I understand ember-auto-import correctly, the FastBoot.require is not used for importing from 3rd packages in FastBoot, because add-on handles this on its own via webpack.
This is definitely true, but today the distinction between import and not means that we never accidentally ship things inside the runtime assets that are from FastBoot.require
. Changing to use import here means that we need to make sure that those imports aren’t bundled into the main (non-“FastBoot only”) assets.
You mean that await import('...') at top level is not supported today in Ember?
No, not specifically. I mean that using import()
should never return the imported thing synchronously. It returns a promise, and when that promise resolved the imported thing is returned. Not all codepaths that exist today allow absorbing a promise (e.g. service methods, any objects init, etc). I don’t think this is fundamentally fatal, just requires some thought.
I'm not sure if I understand correctly, but the ember-auto-import seems to already know how to dynamically import 3rd packages both when it is running in browser or in FastBoot.
That’s great! My point was more that we need to make sure that we don’t accidentally bloat our runtime assets with SSR specific code.
Based on the exploration I've made ef4/ember-auto-import#266 with @ef4's help, I'm going to update the proposal to "drop" FastBoot.require
for 3rd dependencies, and use it only for Node.js builtins.
There are already add-ons, like ember-useragent
, which use ember-auto-import
instead of FastBoot.require
.
There are also some add-ons, like ember-fetch
, which can not use EAI until Embroider I assume, until that this will be just a proposal.
However, because ember-fetch
is the last add-on in our stack, which requires npm install
on build/deploy, I've proposed an interim change ember-cli/ember-fetch#493. This will allow us to remove npm install
from ember-fastboot/fastboot-s3-downloader#18 and avoid issues like ember-fastboot/fastboot#221.
A few updates to the earlier questions in this thread based on discussion around the embroider v2 addon format RFC:
How do we absorb the asynchrony introduced?
This is why the importSync
macro is in that RFC. It gives us one declarative way to handle this problem wherever it comes up. Also, as an optional extension we could choose to allow top-level await inside macroCondition
-guarded conditionals, because that's much easier to implement that allowing top-level await everywhere.
Since inside an Ember application there is already a global variable named require (defined here), I don't see how you'd gain access to the Node-land require without having it exposed in some way (e.g. FastBoot.require).
The point would be eliminating the distinction. There would no longer be any way to access node's require
. Everything that's allowed to be used in fastboot would be packaged into the build already, so that Ember's require
knows how to hand it back to you.
Do we need to make ember-auto-import (and embroider) aware of this specific pattern, so that it knows that the shim it uses to implement dynamic requires should use node-land vs in-app systems?
No. Realize that for both auto-import and embroider the distinction already doesn't exist. Trying to maintain the distinction is huge source of pain and complexity.
Hello,
according to the upcoming major release, would it make sense to drop custom FastBoot import system and use what JavaScript has by spec?
Some benefits could be
3rd dependencies
For importing external modules from 3rd packages the ember-auto-import could be used.
Add dependencies as usual
npm install --save-dev ldclient-node ldclient-js
.Use dynamic import to conditionally import modules in FastBoot / browser.
Node.js built-in modules
FastBoot.require
will be still used for importing Node.js builtins at runtime, however it could be marked "private", because onlyember-auto-import
should use it at compile time, add-ons/applications will use native JavaScript.I'm not sure if I'm missing something that could prevent from dropping
FastBoot.require
now.Thanks