aws-amplify / amplify-js

A declarative JavaScript library for application development using cloud services.
https://docs.amplify.aws/lib/q/platform/js
Apache License 2.0
9.42k stars 2.12k forks source link

Modules not correctly marked as having side effects #3097

Closed nihakue closed 4 years ago

nihakue commented 5 years ago

Is your feature request related to a problem? Please describe. Amplify modules have a side effect (registration), but do not provide a sideEffects config in their package.json. The effect of this is that when doing something like this:

import Amplify from '@aws-amplify/core'
import Auth from '@aws-amplify/auth'
import Analytics from '@aws-amplify/analytics'

Amplify.configure({
  Auth: {...},
  Analytics: {...},
});

when using tree-shaking, Auth will potentially get tree-shaken out of the bundle because it's only being imported for its side-effect.

Describe the solution you'd like Add sideEffects config to package.json or make modules pure.

jkeys-ecg-nmsu commented 5 years ago

If you're using modular imports, why wouldn't you do this?

Auth.configure({...});
Analytics.configure({...});
haverchuck commented 5 years ago

@nihakue - Did the response from @jkeys-ecg-nmsu help?

nihakue commented 5 years ago

Thanks for the quick responses,

That's a fine workaround, but the fact remains that Amplify modules claim to be ES6 modules (they set module in package.json) but have effectful imports. Most places in the Amplify documentations tell you to configure via Amplify.configure, and there are blog posts high in the Google search results that tell you to do this.

If this is not expected to work, maybe Amplify should warn/error when you try it. If it is expected to work, I think it should support (or at least not break when) tree shaking. The way to support it is with "sideEffect" in package.json

ChristianDavis commented 5 years ago

This is very confusing to figure out.

Auth.configure({Auth: {...}}) 

works, but should at least be documented.

Amplifiyer commented 4 years ago

We have added sideEffects config sideEffects:false in all of Amplify modules after removing the side effects from index files. These changes are now available in Amplify V3

github-actions[bot] commented 3 years ago

This issue has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels or Discussions for those types of questions.