embroider-build / ember-auto-import

Zero config import from npm packages
Other
361 stars 110 forks source link

Fix imports with a query part #603

Closed simonihmig closed 6 months ago

simonihmig commented 7 months ago

With the new allowAppImports feature, apps can import resources that are handled with custom webpack loaders. Some can allow you to customize their behavior by passing query params, like import image from './images/image.jpg?size=1024.

This PR is fixing two related issues:

simonihmig commented 7 months ago

I haven't yet looked into adding tests for this. I guess for covering imports of non-JS files, including query params, we would need to add a simple webpack loader in a scenario test? Like on that returns the query as the generated module's export, so we can assert that the loader is correctly invoked and the query stuff works? Should that be a separate scenario? Or any other suggestions?

simonihmig commented 7 months ago

Btw, I worked on these changes as part of making https://github.com/simonihmig/ember-responsive-image/pull/442 work with ember-auto-import, and it seems this is indeed the final missing piece, as the things that worked under Embroider are now also passing (locally) under eai! :tada:

ef4 commented 7 months ago

This is looking reasonable. For tests, I think you can extend these where we test allowAppImports in general.

Adding a loader would be OK, but it also might be enough to use built-in webpack features like Asset Modules.

simonihmig commented 6 months ago

Added tests, this should be good now for another review! @ef4 @mansona