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

Add web component dev guide #77

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cbeach47
Copy link
Contributor

@cbeach47 cbeach47 commented Sep 8, 2023

No description provided.

Comment on lines +32 to +34
### Lerna
https://lerna.js.org/
https://github.com/ChristianMurphy/lerna-webjar
Copy link
Member

Choose a reason for hiding this comment

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

hmmm, a couple thoughts on this:

  1. lerna is (largely) being replaced by native monorepo/workspace features in npm and yarn
  2. lerna or workspaces are mostly useful when there are many web components that need to be managed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you see a value-add in keeping multiple web components bundled in the same repository? If we could go down to a single web component per repo, and then drop lerna, that'd definitely help simplify the toolset and allow web components the flexibility to be revised in isolation

Copy link
Member

Choose a reason for hiding this comment

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

Do you see a value-add in keeping multiple web components bundled in the same repository?

Yes, but there are trade offs.
If the components are using the same technologies and interdependent.
Having a monorepo can help ensure we have consistency across toolchains, dependency versions, and release timing.

If the components are not using the same technologies and interdependent.
The value proposition for monorepos breaks down, and it can be more trouble than it's worth.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

makes sense. I'll note there are two flavors of build flows (npm/yarn & lerna)

Copy link
Member

@ChristianMurphy ChristianMurphy Sep 8, 2023

Choose a reason for hiding this comment

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

Lerna is for most intents and purposes deprecated, replaced by native monorepo workspace support in package managers, removing it from uPortal web components is something I hope to do sometime.
These days npm for a single repo and npm workspaces for a monorepo would be a go-to.

Yarn and pnpm could also be options, but lately they've been trending towards a new style of module loading called plug and play, which can break random modules and can be challenging to debug without fairly in depth knowledge of node js packaging conventions and module loading rules. I'd steer clear of these to keep the guide more beginner friendly.

Comment on lines +48 to +51
### ESLint
https://eslint.org/

Linter for Javascript.
Copy link
Member

Choose a reason for hiding this comment

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

ESLint starts with all rules off, and without framework specific guidance.
It would probably be good to recommend adding

{
  "root": true,
  "parser": "@typescript-eslint/parser",
  "extends": [
     "eslint:recommended",
     "plugin:prettier/recommended",
     "plugin:@typescript-eslint/recommended",
     "plugin:wc/best-practice",
     "plugin:lit/recommended"
  ]
}

Comment on lines +43 to +46
### Typescript
https://www.typescriptlang.org/

Type checking.
Copy link
Member

Choose a reason for hiding this comment

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

It may also be nice to include https://github.com/plantain-00/type-coverage#readme and encourage a minimum amount of typing (I personally prefer 100% types, but 90% could be a good baseline)

Comment on lines +53 to +56
### StyleLint
https://stylelint.io/

Linter for CSS.
Copy link
Member

Choose a reason for hiding this comment

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

Comment on lines +68 to +70
### Editor Config

Consistent code style in the IDE.
Copy link
Member

Choose a reason for hiding this comment

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

This feels optional with prettier, fine with keeping it though

Comment on lines +58 to +61
### Remark
https://github.com/remarkjs/remark-lint

Linter for markdown.
Copy link
Member

Choose a reason for hiding this comment

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

Comment on lines +10 to +12
Linters
v
Pre-commit hooks
Copy link
Member

Choose a reason for hiding this comment

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

Ideally pre-commit hooks would trigger the linters automatically

Comment on lines +14 to +18
Deployed to Sonatype
v
Bundled as a webjar
v
Deployed to Maven
Copy link
Member

Choose a reason for hiding this comment

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

Aren't Sonatype and Maven central the same?

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.

2 participants