Skip to content
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

[2.19] Update documentation on Dart library directive to include non-name libraries #4314

Closed
kevmoo opened this issue Oct 24, 2022 · 16 comments · Fixed by #4390
Closed

[2.19] Update documentation on Dart library directive to include non-name libraries #4314

kevmoo opened this issue Oct 24, 2022 · 16 comments · Fixed by #4390
Assignees
Labels
a.language Relates to the Dart language tour dev.packages Relates to package publishing e1-hours Can complete in < 8 hours of normal, not dedicated, work p3-low Valid but not urgent concern. Resolve when possible. Encourage upvote to surface.

Comments

@kevmoo
Copy link
Member

kevmoo commented Oct 24, 2022

See dart-lang/language#1073

Ask @srawlins for clarification

@kevmoo kevmoo added the a.language Relates to the Dart language tour label Oct 24, 2022
@kevmoo kevmoo added this to the Next stable release milestone Oct 24, 2022
@kevmoo kevmoo changed the title Update documentation on Dart library directive to include non-name libraries [2.19] Update documentation on Dart library directive to include non-name libraries Oct 24, 2022
@parlough parlough added p3-low Valid but not urgent concern. Resolve when possible. Encourage upvote to surface. docs e1-hours Can complete in < 8 hours of normal, not dedicated, work dev.packages Relates to package publishing labels Oct 24, 2022
@srawlins
Copy link
Member

To give a little summary here, because there's really nothing to this, haha:

We already have a "library directive" which can be written above imports, like library my_library;. You can use this identifier in part files (in an older style), or hang annotations off it (like @deprecated), or hang doc comments off it (with /// comment). The new feature is just that you don't need to spell out a name; you can write library;, hang annotations and doc comments off of that, and no need to write the name.

Previously the name was used in part files, and in mirrors, both of which are older style / soft-deprecated.

So an exciting use for this feature is really just some code like

/// A really great test library.
@TestOn('browser')
library;

...

@mit-mit
Copy link
Member

mit-mit commented Oct 25, 2022

I think the documentation impact might be simply checking if we have any library directives in our docs that have annotations or doc comments, and if so, update those to use use the new library; syntax?

@MaryaBelanger MaryaBelanger self-assigned this Oct 25, 2022
@kevmoo
Copy link
Member Author

kevmoo commented Oct 25, 2022

@mit-mit – likely. Just good to double-check.

@MaryaBelanger
Copy link
Contributor

MaryaBelanger commented Nov 17, 2022

I have a question: Is there a reason to explicitly name libraries at all anymore?
Should we just completely discourage naming libraries and only ever show examples of library; unnamed?

One note in the docs says:

"When the library directive isn’t specified, a unique tag is generated for each library based on its path and filename."

I'm assuming that is now the case for unnamed library; declarations as well, so it seems like there's no reason to name them.

It also seems like the only use for named libraries might be part of, which is old and not used anyway @srawlins?. We could get rid of this Effective Dart section, since we literally say we recommend avoiding it in another note.

In another thread Lasse says: "Giving a library a name is only really useful for avoiding duplicate imports with different paths, so most libraries do not use the declaration." But if that's really the only reason for naming libraries, wouldn't a unique URI bypass that anyway?

I'm asking because information about the library directive is spread out all over the place and it's pretty inefficient to write "you can name them but don't need to unless you want to use part of, which you shouldn't really be doing anyway" everywhere it's mentioned.

@kevmoo
Copy link
Member Author

kevmoo commented Nov 17, 2022

@MaryaBelanger – not really.

If you're using dart:mirrors – maybe. But we discourage that.
You don't need library for parts, either. You SHOULD use the URI of the library instead of the library name.

The ONLY (good) reason for library is /// doc comments and/or @annotations.

@kevmoo
Copy link
Member Author

kevmoo commented Nov 17, 2022

@MaryaBelanger
Copy link
Contributor

MaryaBelanger commented Nov 17, 2022

Ok might be a little drastic but I'm going to add an Effective Dart: Usage section that says something like "DO only use the library directive for doc comments and annotations" (if this is objectionable lmk). It'd be really nice to point to one place for that info (that's not the linter docs, which is the most complete source atm since it's already updated for 2.19)

@kevmoo
Copy link
Member Author

kevmoo commented Nov 18, 2022

Please do! Then we should update the lint to reference that this is a style guide "thing" – @srawlins @pq

@MaryaBelanger
Copy link
Contributor

MaryaBelanger commented Nov 18, 2022

There's already a linter rule (that words it much better!): https://dart.dev/tools/linter-rules#unnecessary_library_directive

Avoid library directives unless they have documentation comments or annotations

Though maybe "DON'T" is the better word, for "things that are almost never a good idea"

@kevmoo
Copy link
Member Author

kevmoo commented Nov 18, 2022

Yeah there's a linter rule. That's great. But we should update the style guide, too.

@mit-mit
Copy link
Member

mit-mit commented Nov 21, 2022

cc @munificent

@munificent
Copy link
Member

Honestly, it's probably not even worth adding to "Effective Dart". There are an infinite number of bad things we could tell users not to do but if the bad thing takes effort to do and doesn't have any even perceived upside, we don't usually tell them not to because they just... won't. :)

We can add a guideline against it if others think it's useful, but I'm always a little worried about "Effective Dart" getting so many rules that people just tune it out entirely.

@MaryaBelanger
Copy link
Contributor

MaryaBelanger commented Dec 2, 2022

There's already a linter rule (that words it much better!): https://dart.dev/tools/linter-rules#unnecessary_library_directive

So this linter rule cites the existing Effective Dart: Documentation section "DO use library directives if you want to document a library and/or annotate a library." (my mistake, that is not actually an Effective Dart section on any of the pages)

But I noticed there's not a linter rule that's just like unnecessary_library_name or something. Should that be a thing too @srawlins ?
I'm asking because of this line (that was already in the docs before I started editing):

A unique tag is generated for each library based on its path and filename. Naming libraries overrides their intrinsic URI, which can actually make it harder for tools to physically find the main library file you’re referring to.

I feel like this is a different concern than what the unnecessary_library_directive lint addresses.

There's also this lint, library_names which at the very least is irrelevant now because naming libraries is useless. Should it be removed, or changed to focus on useless names and not the syntax of names?

@kevmoo any thoughts since you added the lint?
(This isn't blocking my work or anything, just wondering/keeping notes for the future)

@MaryaBelanger
Copy link
Contributor

MaryaBelanger commented Dec 2, 2022

@munificent Good point.

  1. I will remove the "DON'T use library directives unless..." section I added. (especially since that content is already more or less completely covered in the existing "CONSIDER writing library level doc comments" section).

  2. I want to keep the other section I added, "DON'T explicitly name libraries", just in case there is a linter rule to be added (see my previous comment). But, I'll move it to Effective Dart: Style, instead of Usage, since that's what linter rules point to.

    If a linter rule for "don't name libraries" doesn't get created, I'll remove it completely.

  3. Idk why I didn't think of this before, but I think the minutiae of library directive names could actually be a short subsection under Libraries and visibility, so I'm going to add that.

How does that sound?

Done, here: dbac48c

@pq
Copy link
Member

pq commented Dec 6, 2022

LGTM.

If a linter rule for "don't name libraries" doesn't get created, I'll remove it completely.

Tracking issue: dart-lang/sdk#58950.

atsansone pushed a commit that referenced this issue Dec 20, 2022
fixes #4314 

staged: https://lib-dir-docs.web.app

Changes:

- Added [code
sample](https://lib-dir-docs.web.app/guides/language/effective-dart/documentation#consider-writing-a-library-level-doc-comment)
to existing library-level doc comment section
- Added [Don't use library directives unless attaching doc comments or
annotations](https://lib-dir-docs.web.app/guides/language/effective-dart/usage#dont-use-library-directives-unless-attaching-doc-comments-or-annotations)
section
- Added [Don't explicitly name
libraries](https://lib-dir-docs.web.app/guides/language/effective-dart/usage#dont-explicitly-name-libraries)
section
- Reduced [`part
of`](https://lib-dir-docs.web.app/guides/language/effective-dart/usage#do-use-strings-in-part-of-directives)
section
- Removed / modified unnecessary notes about the library directive on
the [Creating
packages](https://lib-dir-docs.web.app/guides/libraries/create-library-packages)
page

I thought about completely removing the `part of` section in Effective
Dart: Usage, in favor of the more general new section "Don't explicitly
name libraries", because it's not even recommended to use `part of` at
all anymore.

The new section could add a code example similar to the one for `part
of` that more generally illustrates "Use URI strings to refer to
libraries", and the linter rule could point to that section instead.

Thoughts? @kevmoo @pq etc.

Co-authored-by: Parker Lougheed <parlough@gmail.com>
Co-authored-by: Kevin Moore <kevmoo@users.noreply.github.com>
@MaryaBelanger
Copy link
Contributor

Fixing PRs merged into v2.19

parlough added a commit that referenced this issue Jan 23, 2023
fixes #4314

Changes:

- Added [code
sample](https://lib-dir-docs.web.app/guides/language/effective-dart/documentation#consider-writing-a-library-level-doc-comment)
to existing library-level doc comment section
- Added [Don't use library directives unless attaching doc comments or
annotations](https://lib-dir-docs.web.app/guides/language/effective-dart/usage#dont-use-library-directives-unless-attaching-doc-comments-or-annotations)
section
- Added [Don't explicitly name
libraries](https://lib-dir-docs.web.app/guides/language/effective-dart/usage#dont-explicitly-name-libraries)
section
- Reduced [`part
of`](https://lib-dir-docs.web.app/guides/language/effective-dart/usage#do-use-strings-in-part-of-directives)
section
- Removed / modified unnecessary notes about the library directive on
the [Creating
packages](https://lib-dir-docs.web.app/guides/libraries/create-library-packages)
page

I thought about completely removing the `part of` section in Effective
Dart: Usage, in favor of the more general new section "Don't explicitly
name libraries", because it's not even recommended to use `part of` at
all anymore.

The new section could add a code example similar to the one for `part
of` that more generally illustrates "Use URI strings to refer to
libraries", and the linter rule could point to that section instead.

Thoughts? @kevmoo @pq etc.

Co-authored-by: Parker Lougheed <parlough@gmail.com>
Co-authored-by: Kevin Moore <kevmoo@users.noreply.github.com>
parlough added a commit that referenced this issue Jan 23, 2023
Fixes #4314

Changes:

- Added [code
sample](https://dart.dev/guides/language/effective-dart/documentation#consider-writing-a-library-level-doc-comment)
to existing library-level doc comment section
- Added [Don't use library directives unless attaching doc comments or
annotations](https://dart.dev/guides/language/effective-dart/usage#dont-use-library-directives-unless-attaching-doc-comments-or-annotations)
section
- Added [Don't explicitly name
libraries](https://dart.dev/guides/language/effective-dart/usage#dont-explicitly-name-libraries)
section
- Reduced [`part
of`](https://dart.dev/guides/language/effective-dart/usage#do-use-strings-in-part-of-directives)
section
- Removed / modified unnecessary notes about the library directive on
the [Creating
packages](https://dart.dev/guides/libraries/create-library-packages) page

Co-authored-by: Parker Lougheed <parlough@gmail.com>
Co-authored-by: Kevin Moore <kevmoo@users.noreply.github.com>
parlough added a commit that referenced this issue Jan 24, 2023
Fixes #4314

Changes:

- Added [code
sample](https://dart.dev/guides/language/effective-dart/documentation#consider-writing-a-library-level-doc-comment)
to existing library-level doc comment section
- Added [Don't use library directives unless attaching doc comments or
annotations](https://dart.dev/guides/language/effective-dart/usage#dont-use-library-directives-unless-attaching-doc-comments-or-annotations)
section
- Added [Don't explicitly name
libraries](https://dart.dev/guides/language/effective-dart/usage#dont-explicitly-name-libraries)
section
- Reduced [`part
of`](https://dart.dev/guides/language/effective-dart/usage#do-use-strings-in-part-of-directives)
section
- Removed / modified unnecessary notes about the library directive on
the [Creating
packages](https://dart.dev/guides/libraries/create-library-packages) page

Co-authored-by: Parker Lougheed <parlough@gmail.com>
Co-authored-by: Kevin Moore <kevmoo@users.noreply.github.com>
parlough added a commit that referenced this issue Jan 24, 2023
Fixes #4314

Changes:

- Added [code
sample](https://dart.dev/guides/language/effective-dart/documentation#consider-writing-a-library-level-doc-comment)
to existing library-level doc comment section
- Added [Don't use library directives unless attaching doc comments or
annotations](https://dart.dev/guides/language/effective-dart/usage#dont-use-library-directives-unless-attaching-doc-comments-or-annotations)
section
- Added [Don't explicitly name
libraries](https://dart.dev/guides/language/effective-dart/usage#dont-explicitly-name-libraries)
section
- Reduced [`part
of`](https://dart.dev/guides/language/effective-dart/usage#do-use-strings-in-part-of-directives)
section
- Removed / modified unnecessary notes about the library directive on
the [Creating
packages](https://dart.dev/guides/libraries/create-library-packages) page

Co-authored-by: Parker Lougheed <parlough@gmail.com>
Co-authored-by: Kevin Moore <kevmoo@users.noreply.github.com>
@atsansone atsansone removed the docs label Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a.language Relates to the Dart language tour dev.packages Relates to package publishing e1-hours Can complete in < 8 hours of normal, not dedicated, work p3-low Valid but not urgent concern. Resolve when possible. Encourage upvote to surface.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants