Skip to content

Post-microbundle updates#174

Merged
mdespuits merged 11 commits intomasterfrom
post-microbundle-updates
May 18, 2020
Merged

Post-microbundle updates#174
mdespuits merged 11 commits intomasterfrom
post-microbundle-updates

Conversation

@mdespuits
Copy link
Copy Markdown
Contributor

@mdespuits mdespuits commented May 15, 2020

Adds CONTRIBUTING.md

Github's official file used for contributing guidelines. Simplifies the main README quite a lot, too

Updates the example App

  • Adds Tailwind from CDN for nicer looking examples
  • Fixes bugs found through TypeScript as console error logs

Kapture 2020-05-15 at 11 51 27

Updates the hygen generated code

Finding component files in Rover has always been a bit difficult due to everything being called index.js or style.css. For future components, I'm proposing switching to use index.ts for simple exporting and Component.tsx and Component.module.css for easier to file finding.

Matthew Wells added 6 commits May 15, 2020 09:09
Switches format of component director structure to the following:

$ yarn generate-component Select
src/components/Select
├── README.md
├── Select.css
├── Select.tsx
├── index.ts
├── story.js
└── test.js

This makes
@mdespuits mdespuits added the documentation Helping us not forget label May 15, 2020
@mdespuits mdespuits force-pushed the post-microbundle-updates branch from dbd1337 to 653794b Compare May 15, 2020 17:45
Try building in pull request once

Split install and deploy steps for visibility

Prefix with yarn in npm scripts

gh-pages got dropped from de dependencies

Trying custom Github action for Github Pages
@mdespuits mdespuits force-pushed the post-microbundle-updates branch from 653794b to 9f11d38 Compare May 15, 2020 18:05
run: |
yarn install
yarn build-storybook
yarn storybook:build
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was the real problem. build-storybook does not build to docs by default

@mdespuits mdespuits force-pushed the post-microbundle-updates branch from 1329384 to f771b3c Compare May 15, 2020 19:27
Copy link
Copy Markdown
Contributor

@sebastianvera sebastianvera left a comment

Choose a reason for hiding this comment

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

Really nice work, I'm happy with the new files' structure, I find it annoying to have the component definition in the index file. It is also nice to see tailwind 🙌 (I know it's just part of the example, but anyway)

🚀

Comment thread .eslintrc
Comment thread _templates/component/new/Component.module.css.t Outdated
Comment thread CODE_GUIDELINES.md
|-- test.js # Used with an npm command
`-- story.js # Used by storybook, separate npm command
|-- index.ts # Re-exports from Modal.tsx
|-- Modal.tsx # Core component file, imports Modal.css
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bikeshedding:
I know I read a couple of places that newer recs were to use .js and not .jsx, even with JSX in the files.
The argument boiled down to "everything is transpiled anyway, and not having to rename files is better ergonomics".

Would that make sense with TS files too (skip the trailing "x"?)
I don't mind much either way, but I have a slight preference for using the same extension for all.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have not looked into this much, but when I leave off the s for TS components, TS definitely complains loudly. With the x, it happily parses it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@pixelbandito Ah, here's an explanation for this: https://www.typescriptlang.org/docs/handbook/jsx.html#basic-usage. For explicit TS, we need to use the .tsx extension to enable JSX for TypeScript components

Comment thread CONTRIBUTING.md
@mdespuits mdespuits merged commit c31f21d into master May 18, 2020
@mdespuits mdespuits deleted the post-microbundle-updates branch May 18, 2020 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Helping us not forget

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants