ember-cli / ember-load-initializers

MIT License
14 stars 37 forks source link

Embroider crash when initializer/instance-initializer does not have a default export #277

Open mehulkar opened 3 years ago

mehulkar commented 3 years ago

If you have an initializer that does not have a default export:

export function initialize() {}

// export default { initialize } 

resolveInitializer crashes here:

https://github.com/ember-cli/ember-load-initializers/blob/b69de85ab66bfa0d857b8165e5bbc7769fba8382/addon/index.ts#L15-L16

This function only seems to be called when I enable Embroider builds, not in regular ember-cli builds, so I'm not sure if this is intentional or what the expectations are.

ember-cli could be stricter and require this default export, but if it's intentional to not require the default export, then resolveInitializer should handle this case as well?

mehulkar commented 3 years ago

It seems that ember-cli may not be using the default export, and it's just using the named export, but I'm not sure if the object here is the module or not.

https://github.com/emberjs/ember.js/blob/ec49e0eee542791ca6916a71096988acab9c2ec0/packages/%40ember/engine/index.js#L124

rwjblue commented 3 years ago

This function only seems to be called when I enable Embroider builds, not in regular ember-cli builds, so I'm not sure if this is intentional or what the expectations are.

I don't fully understand what you mean here. Are you saying that in ember-cli's build pipeline that initialize function is never invoked, but in Embroiders it is invoked? Or are you saying the reverse (called in ember-cli pipeline but not in Embroider)?

I believe these are related references:

mehulkar commented 3 years ago

This function only seems to be called when I enable Embroider builds, not in regular ember-cli builds, so I'm not sure if this is intentional or what the expectations are.

I don't fully understand what you mean here.

I was totally wrong here. the function is called in both.

The difference seems to be that when the default export is missing, with regular ember builds, module.default is circular:

{ initialize: [Function: initialize], default: [Circular] }

So it doesn't cause a crash while registering the initializers.

With Embroider, maybe require is being changed, causing the circular structure not to happen?

mehulkar commented 3 years ago

I've made simple repro here (see commit history): https://github.com/mehulkar/embroider-initializer-crash. The easiest workaround is to just add the default exports into my initializers, but I think resolveInitializer should explicitly throw when module.default is undefined, so that adopting Embroider doesn't cause this, kind of hard to track problem.

rwjblue commented 3 years ago

Right, you are describing the same thing that is described in the issues I linked in my initial comment. If you use the flag to disable the default export munging in loader.js you will have the same behavior in both worlds.

mehulkar commented 3 years ago

How do I use this flag? I looked ovr that issue but don't see how to do it from the standpoint of ember-cli. Not sure where loader.js gets invoked.