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

docs(publishing): update general docs on publishing #1359

Merged
merged 13 commits into from
Mar 11, 2024

Conversation

christian-bromann
Copy link
Member

Currently our documentation around publishing docs in not very well structured and content is scattered among different pages. This caused confusions in the ecosystem. This patch attempts to clean up our docs by restructuring the content and put an emphasis on the distribution use cases: lazy loading and standalone Stencil modules. To summarize the changeset:

  • I removed content on the topic of publishing components within the output targets docs and moved parts of it into the general publishing docs
  • I updated the structure of our publishing docs with focus on the use cases
    • moved the recommendation for package.json exports based on the use case
    • added a section on consideration to explain when a certain use case might be favorable over the other

@christian-bromann christian-bromann requested a review from a team as a code owner February 22, 2024 02:13
@christian-bromann christian-bromann requested review from rwaskiewicz and alicewriteswrongs and removed request for a team February 22, 2024 02:13
Copy link

vercel bot commented Feb 22, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
stencil-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 11, 2024 4:27pm

docs/guides/publishing.md Outdated Show resolved Hide resolved
docs/guides/publishing.md Outdated Show resolved Hide resolved
docs/guides/publishing.md Outdated Show resolved Hide resolved
docs/guides/publishing.md Outdated Show resolved Hide resolved
docs/guides/publishing.md Outdated Show resolved Hide resolved
docs/guides/publishing.md Outdated Show resolved Hide resolved
docs/guides/publishing.md Outdated Show resolved Hide resolved
docs/guides/publishing.md Outdated Show resolved Hide resolved
@christian-bromann
Copy link
Member Author

Thanks for the feedback. I applied the updates accordingly.

Copy link
Contributor

@rwaskiewicz rwaskiewicz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I had a few more asks after taking some time between reads. LMK if you have any questions

Comment on lines +60 to +64
// or extend custom element via
class MyCustomComponent extends MyComponent {
// ...
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a use case we support? Extending a component built by Stencil?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, for example, let's say we publish a create-stencil component, you can change functionality by extending the class and overwriting component methods, e.g.:

<script type="module">
  import { MyComponent } from 'https://unpkg.com/[email protected]/dist/components/index.js'
  class ChangedComponent extends MyComponent {
    getText() {
      return 'a new component!';
    }
  }

  customElements.define('my-component', ChangedComponent);
  customElements.define('my-component-orig', MyComponent);
</script>
<my-component first="foo" last="bar"></my-component>
<my-component-orig first="foo" last="bar"></my-component-orig>

This will result in:

Screenshot 2024-02-28 at 10 16 25 AM

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow I didn't know that worked! I guess I hadn't tried it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah extending the compiled class output will work, but obviously wouldn't be able to add any additional reactivity (states, props, watchers, etc.) at this time.

docs/guides/publishing.md Outdated Show resolved Hide resolved
docs/guides/publishing.md Outdated Show resolved Hide resolved
@@ -128,32 +128,6 @@ _default: `false`_

Setting this flag to `true` will cause file minification to follow what is specified in the [Stencil config](../config/01-overview.md#minifyjs). _However_, if [`externalRuntime`](#externalruntime) is enabled, it will override this option and always result in minification being disabled.

## Consuming Custom Elements

By default, the custom elements files will be written to `dist/components/`. This directory can be configured using the output target's [`dir`](#dir) config.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like we lost some of this information in the transition, can we integrate this back into the docs?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some sections back in Publishing > Use Cases > Standalone

Copy link
Contributor

@alicewriteswrongs alicewriteswrongs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just had a few questions and suggestions, looks like a step forward overall!

docs/guides/publishing.md Outdated Show resolved Hide resolved
Comment on lines +60 to +64
// or extend custom element via
class MyCustomComponent extends MyComponent {
// ...
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow I didn't know that worked! I guess I hadn't tried it

Comment on lines +73 to +82
"exports": {
".": {
"import": "./dist/components/index.js",
"types": "./dist/components/index.d.ts"
},
"./my-component": {
"import": "./dist/components/my-component.js",
"types": "./dist/components/my-component.d.ts"
}
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not something we should do now, but it would be nice for supporting this usage of exports with DCE if we could auto-generate a JSON blob for all the components, so that users didn't have to automatically keep it in sync - imagine the nightmare of doing that if you had 200 components or something like that

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, sounds like a great idea!

Comment on lines +102 to +104
:::note
If you are distributing both the `dist` and `dist-custom-elements`, then it's best to pick one of them as the main entry depending on which use case is more prominent.
:::
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By this you mean the file path in the main field in package.json? There's a main field shown above for dist but it doesn't show one for DCE, might be helpful to have that more explanation of what setting the main field would look like for DCE

(possible I'm also misunderstanding something here haha)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left this out intentionally as I don't know if it is a good idea to define a main entry point DCE. main is usually only used for CJS environments. We don't export DCE as CJS module. I am sure this can be configured but don't think anyone would want to import CJS to then re-transpile back to ESM which is what is supported in the browser.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok that sounds fine, I guess if the user is going to distribute both then we can rely on them a bit to figure out how to do that correctly

@christian-bromann christian-bromann requested review from tanner-reits and removed request for rwaskiewicz March 5, 2024 18:11
Comment on lines +60 to +64
// or extend custom element via
class MyCustomComponent extends MyComponent {
// ...
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah extending the compiled class output will work, but obviously wouldn't be able to add any additional reactivity (states, props, watchers, etc.) at this time.

@christian-bromann christian-bromann merged commit 53c298e into main Mar 11, 2024
5 checks passed
@christian-bromann christian-bromann deleted the cb/update-publishing-docs branch March 11, 2024 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants