embroider-build / ember-auto-import

Zero config import from npm packages
Other
360 stars 109 forks source link

QUnit chunks are created when creating a production build of a fresh Ember app #525

Open bertdeblock opened 2 years ago

bertdeblock commented 2 years ago

Originally reported here https://github.com/ember-cli/ember-cli/issues/9915.

I'm not 100% sure ember-auto-import is the only culprit, but the issue only happens with ember-auto-import v2. When creating a production build of a fresh Ember v3.28 app that uses ember-auto-import v1, the issue doesn't occur.

VincentMolinie commented 2 years ago

I do have the same issue. It also happens in dev, not sure this is normal but when doing an ember s Qunit is included too. Can I help in anyway ?

runspired commented 2 years ago

This seems like correct behavior. All builds include test assets. With ember-auto-import v2 qunit is now included via it vs via the old test-assets file, but test fils should still be built.

bertdeblock commented 2 years ago

I'm not sure if I'm following. I can't seem to understand why a QUnit chunk should be created for a production build. The chunk is unused as well. Is something importing Qunit in a production build when it shouldn't?

runspired commented 2 years ago

@bertdeblock Because testing production builds is a thing everyone should do (though most don't and I mostly see only addons do this)

void-mAlex commented 2 years ago

having just upgraded to v2 we also saw issues with miragejs and a few others ending up in production build when they are only referenced in test files

we temporarily added the packages to

autoImport: {
  exclude: process.env.EMBER_ENV === 'production' ? ['miragejs'] : [],
}

in order to get around the issue would a reproduction repo help?

ef4 commented 2 years ago

Yes please, a reproduction repo would be great.

bertdeblock commented 2 years ago

I created bertdeblock/ember-auto-import-525-repro. It's a fresh v4.6.0 app. I also included the dist folder which is the result of running ember build --environment=production.

The QUnit chunks are:

void-mAlex commented 2 years ago

I will add that the same result is visible with a 3.28 app once upgraded to auto-import v2.* but not visible with auto-import v1 also still investigating but there may be something happening that might relate to this somehow, with transform plugins no longer registering correctly to strip off data-test attributes from the final build

@ef4 any pointer you can give to what may have catered for this in v1 vs v2? as far as I can tell ember-cli sends all trees on build and somehow that was handled in v1

  toArray() {
    return [
      this.getAddonTemplates(),
      this.getStyles(),
      this.getTests(),  // <----- if I return null here build works as expected
      this.getExternalTree(),
      this.getPublic(),
      this.getAppJavascript(this._isPackageHookSupplied),
    ].filter(Boolean);
  }

I'm still digging and finding my way around the different pieces of the puzzle but hints are greatly appreciated

void-mAlex commented 2 years ago

@bertdeblock Because testing production builds is a thing everyone should do (though most don't and I mostly see only addons do this)

I get that, but how does one ship a version that doesn't deliver 5 MB of test data to actual end users? that is certainly incorrect behaviour at least for us.

bertdeblock commented 2 years ago

@void-mAlex, in my test, the chunk is only created, but not included in index.html, so it won't be requested by the end user. This is the case for your app?

void-mAlex commented 2 years ago

for us it's added into the index.html by the ember cli build process so it's always requested default npm run build script thinking it's got something to do with using it with 3.28?

bertdeblock commented 2 years ago

Could it be that maybe you are importing/including test-related code in the app itself somehow? With a fresh app, I only observed the chunk being made, but not included in the index.html file.

void-mAlex commented 2 years ago

Could it be that maybe you are importing/including test-related code in the app itself somehow? With a fresh app, I only observed the chunk being made, but not included in the index.html file.

I can see the same thing you describe on the 4.6 repro you posted, chunk being generated but not included in the html. with that said I can 100% confirm that test code is not even accidentally referenced anywhere in the app itself, but the chunks themselves are being included in the index.html I'll try and set up a minimal reproduction myself as soon as I get a chance. as I've hinted earlier there is certainly something funky going on as the data-test attributes that were supposed to be stripped by ember-test-selectors also started showing up in production builds after the v2 upgrade of ember-auto-import

edit: it would make more sense why more people are not panicking over this if it only shows up in 3.28 scenario - have a feeling people are either on 4.x with v2 or unable to upgrade due to other issues or it works if you also use embroider (which we don't currently) - will investigate further

void-mAlex commented 2 years ago

so turns out (as far as I can tell) ember-auto-import@2 is behaving correctly and the miragejs and faker packages appearing in the scripts added to the html is due to an addon, that is depended upon by the host, exporting a mirage folder from under the app folder containing some shared factories this was somehow ok under e-a-i@1 and ignored on prod builds

is there any pattern that can be used to share mirage factories across different apps without duplicating code and still be embroider considerate and not use addon lifecycle hooks?