-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BUGFIX] cleanup rollup warnings #6809
Conversation
Asset Size Report for 924ab54 EmberData has not changed in sizeIf any packages had changed sizes they would be listed here. Changeset
Full Asset Analysis
|
} else if (message.code === 'NON_EXISTENT_EXPORT') { | ||
// ignore ts-interface imports | ||
if (message.message.indexOf(`/ts-interfaces/`) !== -1) { | ||
return; | ||
} | ||
} else if (message.code === 'UNRESOLVED_IMPORT') { | ||
if (!this.isDevelopingAddon()) { | ||
// don't print these for consumers | ||
return; | ||
} else { | ||
// make warning actionable | ||
// eslint-disable-next-line no-console | ||
console.log( | ||
chalk.yellow( | ||
`\n\n⚠️ Add ${chalk.white( | ||
message.source | ||
)} to the array returned by externalDependenciesForPrivateModule in index.js of ${chalk.white( | ||
this.name | ||
)}\n\n` | ||
) | ||
); | ||
return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of suppressing these, we should fail the build when developing. This will still have the same result (consumers will not see warnings), but actually forces us to ensure the repo is in the correct shape.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rwjblue I initially did make it throw but figured outputting a clear warning with specific guidance on how to fix it might be enough. I can still throw after this warning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened #6850 to address my concern.
Asset Size Report for 4c6041e EmberData has not changed in sizeIf any packages had changed sizes they would be listed here. Changeset
Full Asset Analysis
|
No description provided.