-
Notifications
You must be signed in to change notification settings - Fork 58
no_entrypoint_imports #985
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
no_entrypoint_imports #985
Conversation
annawatson-wk
left a comment
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.
+10, CI passes
greglittlefield-wf
left a comment
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.
Just a couple small comments, otherwise LGTM!
lib/src/component/aria_mixin.dart
Outdated
| // ignore: unused_shown_name | ||
| import 'package:over_react/over_react.dart' | ||
| show PropDescriptor, PropsMeta; | ||
| import 'package:over_react/over_react.dart' show PropDescriptor, PropsMeta; |
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.
Should this be updated to a src import?
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.
ohh good catch :loading:
lib/src/util/rem_util.dart
Outdated
| import 'package:over_react/src/util/css_value_util.dart'; | ||
| import 'package:platform_detect/platform_detect.dart'; | ||
|
|
||
| import '../../react_dom.dart' as react_dom; |
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.
Could we make this a package: import instead of a relative path?
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.
should I move the react_dom.dart file into src and re-export it from lib?
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.
because that file is lib/react_dom.dart so I couldn't get a src import
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.
Ohh is this the same file as package:over_react/react_dom.dart? Yeah, if you wouldn't mind, I think that'd be better
| ReactElement; | ||
| import 'package:w_common/disposable.dart'; | ||
|
|
||
| import '../../../react_dom.dart' as react_dom; |
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.
Same comment as above
greglittlefield-wf
left a comment
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.
+10
|
@Workiva/release-management-p |
rmconsole-wf
left a comment
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.
+1 from RM
Problem
An entrypoint import can be defined as an import within
lib/srcthat imports a file withinlib/*.dart(the entrypoints of a dart package). These imports are always unnecessary, and can contribute to slower dart build performance.Solution
This PR naively removes every entrypoint import within the repo, and updates the
gha-dart/.../checks.yamlto the latest version. In order to merge this PR a few manual changes must be performed:no_entrypoint_importsadditional-checks option in[email protected]that was added in this PR. This will prevent new entrypoint imports from getting added once this PR mergesOur request to teams is to perform the above tasks, so we can default enable the
no_entrypoint_importscheck for all gha-dart consumers. Please reach out to #support-frontend-dx for any questions or issuesQA
Created by Sourcegraph batch change
Workiva/no_entrypoint_imports.