require all ember modules in one place #2628
Conversation
BlueCutOfficial
force-pushed
the
require-ember
branch
from
0acec73 to
40dacc5
Compare
DescriptionThis PR is part of the basic implementation for Vite support. It's a refactoring step that centralizes the call to @mansona can you explain with more details what you have in mind:
|
| RSVP = module.default; | ||
|
|
||
| // The RSVP module should have named exports for `Promise`, etc, | ||
| // but some old versions do not and provide `RSVP.Promise`, etc. | ||
| if (!('Promise' in module)) { | ||
| module = RSVP; | ||
| } |
There was a problem hiding this comment.
I don't know what version this code was supporting, nor how the assignment in the if block fixes what the comment says, but with this PR: the RSVP module we import is emberSafeRequire('rsvp')?.default; so the content of this block should probably be cleaned up.
There was a problem hiding this comment.
I checked this against the oldest version of Ember that we support (3.16) and RSVP.Promise exists so it's safe to remove 👍
| // eslint-disable-next-line ember/new-module-imports | ||
| module = Ember?.run || module; | ||
| // eslint-disable-next-line ember/new-module-imports | ||
| _backburner = Ember?.run?.backburner || _backburner; |
There was a problem hiding this comment.
Just reverted those blocks so module is assigned before _backburner, because it's the order chosen at the top of the file so it eases the reading.
| // it could happen that runloop is available but _backburner is not exported (dead code) | ||
| // then we need to use our own. | ||
| let module = runloop; | ||
| let _backburner = runloop._backburner; |
There was a problem hiding this comment.
I moved the comment from the old try block here because it mentions a fallback on "our own" _backburner, which I assume corresponds to the default assignment at line 7? The one that used to be on line 13 was rather changing _backburner to something other than "our own" if requireModule('@ember/runloop') was not throwing.
|
@BlueCutOfficial thanks for the updates to this, it all looks good (I just can't approve my own PR 😂) |
2 participants
Description
This PR is starting to split out the work from #2625
This just tries to make sure that there is only one module that is calling requireModule and we can provide alternatives for the different environments isolated to that module 👍
Screenshots