Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Document package types #2246
Document package types #2246
Changes from 28 commits
4aafc87
4655c35
048f7e3
8d2848c
56ecf0d
a724457
e82dbc1
286fd60
35114b0
7bb2648
e9d18f3
ea4774d
77d3643
0b72782
1a8aafe
89302d2
71e12a4
9cbd85b
c0a46c7
217ae18
137d28a
96565ad
35fbad8
b8a7aa6
e08ef2a
db62b30
faeb1f1
bad4cba
2f7b79f
09e501d
b44505b
af864c7
a25c948
31c9018
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Needs an intro. Something like:
Modular can build several different kinds of packages. We refer to each package type as a Modular type, and each has different features. The table below summarises the compatibility of each Modular type with Modular commands and details their unique features.
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.
Done.
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.
Let's keep it example driven with clear commands that are separated from the text so they're easier for a developer to find while scanning the docs. We should also repeat the message that
build
is for deployed assets andstart
is for local development, as not everybody is familiar with these concepts.To build your app for deployment, run:
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.
Done.
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.
To run your app locally on a development server, run start:
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.
Done.
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.
Be precise: use numbered bullet-points and say "two essential differences".
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.
Done, see below.
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.
This phrasing makes it sound like is it the build configs between Webpack and esbuild that are the same; use something like this:
ESM Views are built with the same Webpack or esbuild configuration and support the same functionalities as Modular apps.
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.
Done.
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.
These differences could be a list.
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.
Collated all differences in a ordered list.
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.
I think we should be more dogmatic: The
esm-view
type is used for creating micro-frontends.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.
Done.
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.
The typical vs served case is unclear to the uninitiated. The way this is posed makes these docs more philosophical; to make them active, use code-examples that show how to import an esm-view at run-time and how to serve it standalone (etc!).
That may be too much for this document. Could we create a top-level "Package Types" category at
docs/package-types
where theindex.md
file contains the table at the top of this file and there are specific pages for each type? The existing ESM Views section may need to be rethought if we do that. Can it be nested? I think this structure will make it clearer what Modular is/does.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.
As discussed: will create a section for package types but not nest esm-views
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.
Done.
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.
This paragraph is very wordy and I fear it will get lost. Is there a better way to provide this information? E.g. in a "How to use a CDN for external dependencies" guide or something?
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.
I fear we don't have such a resource. Do you wanna try to simplify this? I would like to convey the message that we have esm views because they can be built in isolation and composed at run-time.
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.
Following my comment about separate pages for each package type, I think each page should have headings for each of the [appropriate?] Modular commands (always in the same order), that explain the detail here and include a code snippet for e.g.
modular build my-view
,modular add --type esm-view
etc. Don't worry about the duplication between pages; this is on purpose: some users will only view one page, and those who view several will quickly learn about the common interface that Modular exposes.Optionally, you could link the answers in the table above to each specific section on the page to provide additional detail, if desired, instead of using longer sentences in the table.
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.
The problem with
add
is that we have--unstable-type
, derived from CRA, that we don't really want to show on the docs. The preferred mode of adding a page is the interactive prompt.Apart from that, I like the idea of having structured information: I can do that for build, start and entry-points.
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.
Done.
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.
Oh yes, I forgot about our
unstable
params. I don't know the history there but it's unintuitive!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.
Don't
package
s also only contain source code in theirsrc
directory? I think the defining feature of thesource
type is: Modularsource
packages contain common source code that can be imported from other packages in the monorepo but cannot be built themselves.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.
The subject of the adversative seems to refer to the "that" in the relative clause and sounds a bit confusing. What do you think of:
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.
I like it!