search

cloudfour / cloudfour.com-patterns

The design system, component library and documentation for cloudfour.com and related projects
https://cloudfour-patterns.netlify.app
24 stars 3 forks source link

Update JavaScript conventions? #653

Closed tylersticka closed 4 years ago

tylersticka commented 4 years ago

Since we're updating our CSS conventions to match our latest best practices, it seems fair to also update our JavaScript conventions to match.

I know this project already includes:

Are there other conventions we should take this opportunity to establish?

Potential questions

Current JavaScript functionality

Our site is pretty simple, so I thought I would summarize the current JavaScript functionality we ship with patterns in case that helps inform what sort of conventions should or should not be present:

Current JavaScript third-party dependencies

There aren't many, but we may want to question these, too:

Goal of this issue

My hope is that this issue will spur discussion, resulting in separate, more actionable issues based on our decisions. Let's close this when it stops serving that purpose.

/CC @cloudfour/dev

gerardo-rodriguez commented 4 years ago

I don't know what TypeScript is, but I hear things. Should we be using it?

I have grown to love the static analysis feedback/linting when writing JavaScript. If I had it my way, we'd use TypeScript. 🙂

From Kent C. Dodds' https://testingjavascript.com Static Analysis Testing JavaScript Applications lesson:

There are a ton of ways your application can break. One of the most common sources of bugs is related to typos and incorrect types. Passing a string to a function that expects a number, or falling prey to a common typo in a logical statement are silly mistakes that should never be made, but this happens all the time.

We could write a comprehensive suite of automated tests for our entire codebase to make certain mistakes like this never happen, but that would likely be too much work and slow development down to be worth the benefit. Luckily for us, there are tools we can use to satisfy a whole category of testing with a great developer experience.

gerardo-rodriguez commented 4 years ago

Should our pattern library include tests for client-side functionality? If so, how?

I'd love for us to embrace more testing as a general rule. Jest would be my first choice. If larger E2E tests are applicable, then Cypress would be my recommendation. This is not an either-or, I would start with Jest first and only add Cypress as needed depending on what tests need to be written. I would expect many Jest tests and a few Cypress tests, for example.

calebeby commented 4 years ago

I have grown to love the static analysis feedback/linting when writing JavaScript. If I had it my way, we'd use TypeScript. 🙂

I agree with Gerardo. I've been using TypeScript on a side-project of mine for a couple years and it has been great to work with.

For me, these are the benefits:

  1. Perfect autocompletion for everything
  2. Really good refactoring. When you change something, it tells you everything that needs to be updated. Things often work the first time in the browser.
  3. Improved onboarding. This is more true for larger projects, so it may not apply here as much, but having types really helps understand how something is working. Being able to hover over a variable and seeing what it is going to be is magical ✨

I think this is a great project to try it out on and see how it goes.

gerardo-rodriguez commented 4 years ago

This project seems like a good fit for little vanilla JS components. Is there a good, standard boilerplate format for those? Classes? Functions that apply stuff to elements? Something else?

I'm curious if @Paul-Hebert has an opinion based on the research he's doing.

I would recommend not using ES6 Classes, if avoidable. Consider factory functions instead. 😄

From The Single Biggest Mistake Programmers Make Every Day:

Pure function > Function > Factory > Class Notice that these things increase in complexity as you move from left to right. Start on the left side and move to the right only as needed.

Other resources:

tylersticka commented 4 years ago

Re: Typescript

I'm game, especially if it makes onboarding easier.

Will this impact client-facing JavaScript only, or does it have some impact on our build scripts, Storybook config, etc.?

Re: Testing

I'd love for us to embrace more testing as a general rule. Jest would be my first choice. If larger E2E tests are applicable, then Cypress would be my recommendation. This is not an either-or, I would start with Jest first and only add Cypress as needed depending on what tests need to be written. I would expect many Jest tests and a few Cypress tests, for example.

I'm certainly open to it. This is an aspect of our process I have very little knowledge of. I don't even really know what the difference between those tools are, beyond one having a UI. 😅

Possibly of interest, possible overkill or outdated: Combining Storybook, Cypress and Jest Code Coverage

Re: Structure

I would recommend not using ES6 Classes, if avoidable. Consider factory functions instead. 😄

This makes me wonder if a good first issue to generate from this discussion might be a rewrite of our elastic textarea functionality (side note: that file's comment makes me laugh). It's the smallest one, but it includes a few common problems (DOM manipulation, updating a thing more than once, etc.), and the type of component it modifies (textareas) already exists in our new library.

That way we could have a nice, simple example for testing out some of these ideas that we could critique before extending to more complex examples?

calebeby commented 4 years ago

TS:

Will this impact client-facing JavaScript only, or does it have some impact on our build scripts, Storybook config, etc.?

Structure: Using the textarea JS as a test/example sounds great! We could also use that as a sort of test of setting up TS and jest/cypress

calebeby commented 4 years ago

Should I open an issue for adding babel/typescript?

tylersticka commented 4 years ago

@calebeby Unless others disagree, it sounds like we have two new issues so far:

Josh68 commented 4 years ago

Another plug for adding TS and type-checking -- we've already seen that it helps in understanding Storybook internals when customizing.

Paul-Hebert commented 4 years ago

Lots of good discussion going on! 🎉

I agree with most of what's been written here. I don't have a ton of TS experience, but I get its benefits and would like the opportunity to work with it more. Same deal with testing.

I also agree with using factory functions over classes where possible.

Current JavaScript functionality

In general I'd like to remove client-side dependencies where possible.

UmbrellaJS: DOM manipulation. Used inconsistently in components.

I'd like to pull this out if possible. It's small but not necessary, and it seems like an additional tool to learn and understand that doesn't add much on top of modern DOM APIs.

PrismJS: Already re-implemented, more performant than Highlight.

I don't remember the details of the re-implementation, but it would be nice to move this highlighting server-side if we haven't already. (One less lib to serve, parse, and run on the front end)

eases: Easing function. Probably better served through design tokens?

I agree on the design tokens.

feature.js: Feature detection.

It's hard to know whether this is necessary without knowing how it's used.

GSAP (only in homepage animation): I don't know if I have the will to completely revise this animation, but we could at least upgrade to the latest version for filesize and performance gains.

That makes sense. I think the complexity of the animation warrants using something like GSAP. If we're not already we should limit the loading of GSAP to the home page so other pages load quicker.

If we were concerned about the perf overhead, I could take a crack at rewriting it as a CSS animation similar to the Rube Goldberg technique but it would be a solid chunk of work (likely a day or two) and some of the easing functions available in JS are impossible in CSS since you only get 2 bezier points.

This project seems like a good fit for little vanilla JS components. Is there a good, standard boilerplate format for those? Classes? Functions that apply stuff to elements? Something else?

I'm curious if @Paul-Hebert has an opinion based on the research he's doing.

It's hard to say without more context! 😅

For starters I want to make sure we're using the same terminology. When I hear component I imagine a small bundle of markup, JS, and CSS that renders a widget on a web page. Is that what you mean as well @tylersticka ?

Assuming that is what we mean there may be benefits to using something like web components or a framework but I'm concerned about over-engineering this.

I think it's helpful to think in terms of components when designing and developing the pattern library, but I don't think we need to actually render them as client-side "components" on the Wordpress site.

I think that unless we're handling complex interactivity, components are more useful for developer quality of life than for the end user experience.

For the most part I'd prefer to render and serve static HTML pages without any component overhead. Ideally CSS and JS would be split into smaller chunks that could be loaded as needed, or bundled together.

Let me know if I'm misunderstanding your question here. If you provide some examples of what you imagine being encapsulated as components I may be able to provide more helpful info.

tylersticka commented 4 years ago

Thanks for your thoughts, @Paul-Hebert!

Re: Prism, I think it's probably still necessary unless the server-rendered options available for PHP/WordPress have dramatically improved in the last year or two.

Re: Home animation, this is limited to the home page already… in fact, it's actually embedded in the SVG object, so it isn't loaded at all if the user can't render SVG for some reason. I don't think it's feasible to move to CSS because we're animating masks, which is not currently possible to do with CSS properties. I'd like to limit the scope of the short-term refactor to updating the version of GSAP: I'd rather save larger changes for new animations in the future, y'know?

tylersticka commented 4 years ago

For starters I want to make sure we're using the same terminology. When I hear component I imagine a small bundle of markup, JS, and CSS that renders a widget on a web page. Is that what you mean as well @tylersticka ?

Assuming that is what we mean there may be benefits to using something like web components or a framework but I'm concerned about over-engineering this.

I think a better word might be "module" or "package" or "function." Apologies for my inconsistent noun usage.

Paul-Hebert commented 4 years ago

Re: Prism, I think it's probably still necessary unless the server-rendered options available for PHP/WordPress have dramatically improved in the last year or two.

Ahh bummer

Re: Home animation, this is limited to the home page already… in fact, it's actually embedded in the SVG object, so it isn't loaded at all if the user can't render SVG for some reason. I don't think it's feasible to move to CSS because we're animating masks, which is not currently possible to do with CSS properties. I'd like to limit the scope of the short-term refactor to updating the version of GSAP: I'd rather save larger changes for new animations in the future, y'know?

Cool, works for me! 👍

I think a better word might be "module" or "package" or "function." Apologies for my inconsistent noun usage.

No worries! Gotcha, in that case I'd just echo other thoughts already expressed in this thread:

I agree the elastic textarea seems like a good first module to test out these ideas 🙂

tylersticka commented 4 years ago

One follow-up side note on the home animation point I mentioned before… there's no reason we couldn't actually forego any updates in the short-term, since we have a self-contained SVG that contains all JavaScript dependencies. Might be an easy thing to push off till later.

tylersticka commented 4 years ago

Closing this issue since it seems to have met the goals described in the description. Feel free to re-open if I'm wrong about that!