| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
Review requested:
|
Sorry, something went wrong.
|
The big question here seems to be whether this is a reference or a guide. I get the feeling we should treat this page as the full reference documentation, while treating the ESM modules page as the "read through guide". As such, we should probably aim to include the full semantics of exports as full reference documentation here, including eg conditional exports. Then rather focus on making the entry points section in the ESM documentation the clear and simple guide version for new users. That is - I think we should focus on making this the technical content, and perhaps less on trying to make it accessible to beginners, and put the beginner focus into the ESM modules documentation page. Let me know if that sounds like a sensible approach to you further. This PR does touch on both which is tricky as it is two jobs in one, unfortunately the PR model doesn't fit that well for docs development! Happy to discuss ways to make it easier too. |
Sorry, something went wrong.
|
Just reading through what we have for the ESM docs now, and of course all the guide info is in the packages page here so the ESM docs actually seem much more like a reference now to me. Of course all the pages are both reference and guide information, but in the past the ESM page always had a flow at the top that you could follow to learn the basics all the way through (or at least that was the attempt). If this package page is going to take that place, then we should move the "package reference" to the bottom of the docs page here rather, and treat the top of this page as more like a guide to getting started. The other option is to make this pure reference, and structure everything by the package.json fields, as pure reference material, do the same for the ESM page, then work to create a separate modules guide. The hard part is splitting up the material that was written the other way around so it becomes quite complex. I don't want to make unnecessary work for anyone though, so won't obstruct progress on what people feel is best. |
Sorry, something went wrong.
|
I've moved the package.json section to the end of the document, PTAL.
@guybedford the documentation of conditional exports is already on the same page, do you mean merge it with the "exports" section? |
Sorry, something went wrong.
There was a problem hiding this comment.
This reordering makes all the difference, thanks @aduh95.
In terms of splitting more of the definitions as references under the package.json fields section, that is something we can do gradually in follow-up PRs to make the guide at the top shorter and as appropriate and makes sense and perhaps we don't even do that though as well.
Sorry, something went wrong.
There was a problem hiding this comment.
This is looking almost good to go to me, it would be nice to get these docs changes in before the next release, so +1 to merging sooner rather than later to keep iterating.
Sorry, something went wrong.
There was a problem hiding this comment.
Can you please point out where you've rewritten sections rather than simply moved them? I'd like to separate the effort of moving the sections from revising them as much as possible, so the history is easier to follow.
Sorry, something went wrong.
Originally what this PR did was:
Rewritten content was added to it based on suggestions from other collaborators in this thread.
You and me both, fella ;) All suggestions were backed by a rationale that seemed reasonable to me, but I have no problem if you prefer we drop it for now. Apologies for the aboves that slipped through (#34970 (comment)), it should be fixed now. |
Sorry, something went wrong.
|
Okay, I reread the new "exports" section and the other one, I understand now that this wasn’t a copy/paste. This looks fine to me once we resolve #34970 (comment). |
Sorry, something went wrong.
There was a problem hiding this comment.
Assuming make lint passes (I see warnings in GitHub) then this looks good to land.
Sorry, something went wrong.
|
I've fixed the linter issue, and squashed the commits so the commit queue can deal with the PR 👍 |
Sorry, something went wrong.
Sorry, something went wrong.
|
this changeset is currently pending the feedback items proposed on #34748 thus not landing on v14.x just yet |
Sorry, something went wrong.
Fixes: nodejs#33143 PR-URL: nodejs#34970 Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Fixes: nodejs#33143 PR-URL: nodejs#34970 Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
| Back | FazBrowse Home | New Git URL |
Adds package.json supported fields documentation to packages.md.
Fixes: #33143
Checklist