ember-fastboot / ember-cli-fastboot

Server-side rendering for Ember.js apps
http://ember-fastboot.com/
MIT License
851 stars 159 forks source link

Fix use of Ember global, breaking Ember 4 #849

Closed simonihmig closed 3 years ago

simonihmig commented 3 years ago

I think it's not ideal to rely on assumptions about the global require() and AMD-style modules, especially in a brave new Embroider world. But after all this is what the RFC706 suggests in the drawbacks section, and should be good enough to fix this for the short term...

Fixes #827

simonihmig commented 3 years ago

Somehow, the first attempt (first commit) of using just require('ember').default didn't work in Ember 3.26 (CI was failing), as that threw an error that the module is not defined (require.entries was an empty object), but it did work for 3.27 and 4.0 (tested locally)! 🤔

The fallback check in the 2nd commit fixes this. But given we don't test across Ember versions in CI yet, the confidence in this change is somewhat lower than I would want. Maybe we have to wait for #848 first?

snewcomer commented 3 years ago

@simonihmig Still investigating but seeing ReferenceError: Ember is not defined at when adding your work to this PR.

https://github.com/ember-fastboot/ember-cli-fastboot/pull/848/checks?check_run_id=3400612883

https://github.com/ember-fastboot/ember-cli-fastboot/pull/848/files#diff-6332b424009db608242427547830fa767850f2b49ae9943dc54c8d43ba0e349bR4