From b9533e887482a091841bc0b20443e543ec0603f2 Mon Sep 17 00:00:00 2001 From: Mason Houtz Date: Sun, 21 Feb 2021 14:14:10 -0800 Subject: [PATCH] updating docs --- client/README.md | 135 ------------------ .../component-design/avoid-using-Galaxy.md | 14 -- .../avoid-using-global-Galaxy.md | 77 ++++++++++ .../src/component-design/javascript-noob.md | 14 -- .../src/component-design/never-use-jquery.md | 6 +- .../providers-and-renderers.md | 2 +- client/docs/src/component-design/readme.md | 71 +++++++++ .../src/component-design/testing-advice.md | 3 - .../component-design/unit-testing/readme.md | 20 +++ .../unit-testing/strategies.md | 42 ++++++ .../unit-testing/writing-tests.md | 73 ++++++++++ .../component-design/vue-component-tips.md | 82 ----------- client/styleguide.config.js | 2 +- 13 files changed, 290 insertions(+), 251 deletions(-) delete mode 100644 client/docs/src/component-design/avoid-using-Galaxy.md create mode 100644 client/docs/src/component-design/avoid-using-global-Galaxy.md delete mode 100644 client/docs/src/component-design/javascript-noob.md create mode 100644 client/docs/src/component-design/readme.md delete mode 100644 client/docs/src/component-design/testing-advice.md create mode 100644 client/docs/src/component-design/unit-testing/readme.md create mode 100644 client/docs/src/component-design/unit-testing/strategies.md create mode 100644 client/docs/src/component-design/unit-testing/writing-tests.md delete mode 100644 client/docs/src/component-design/vue-component-tips.md diff --git a/client/README.md b/client/README.md index e5bee828d32..5a90acd5109 100644 --- a/client/README.md +++ b/client/README.md @@ -164,138 +164,3 @@ terminal this starts for executing Jest tests. yarn run jest-watch Dialog yarn run jest-watch workflow/run - -### Writing a test file - -Jest will try to test any file ending in "\*.test.js". Please place your test -files inside client/src folders right next to whatever files that they are -testing. - -Jest has extensive documentation on the expect API, mocking, and more on the -[official docs page](https://jestjs.io/docs/en/getting-started.html), which will -be your best resource here. - -```javascript -// yourtestfile.test.js - -import { things } from "./yourtestfile.js"; - -describe("some module you wrote", () => { - let transientVariables; - let serviceInstances; - let testData; - - beforeEach(() => { - // setup your code (if necessary) - }); - - afterEach(() => { - // teardown your code (so it doesn't ruin the next test) - }); - - it("should do something or other", () => { - expect(workflowNodeCount()).toBe(5); - }); -}); -``` - -### Testing ~~Suggestions~~ Obligations - -#### Clearly document the intent of your test - -Please remember that these tests are not _for_ you. They're for the people who -come after you. It will be a lot easier to modify, repair and upgrade your code -if they can figure out what you were originally hoping to accomplish. Try to -use as detailed 'expect' statements as possible -- overuse of 'toBeTruthy()' -for example, can hide the intent of your test. - -Add a couple of comments. Use variable names that mean something. Nobody's -code is as self-documenting as they believe it to be. - - -#### Only test the public API that you define (carefully!) - -Internal implementations come and go with library upgrades and new tech. But -the point of the unit test is to make sure your units work as designed.... -which means you need to... you know... design your code to work in units. - -Separate your concerns and identify the developer-facing methods and functions -you expect them to use. Test THOSE. Everything else should probably be -considered an implementation detail. - -The other side of the same coin is to test *only* the unit in question. If your -component has a model that uses a service that touches Vuex, which then uses -Axios to fetch some data -- don't test all that at once. Break things apart and -mock functionality to isolate testing to units. End to end testing is a -separate thing that shouldn't be attempted using spec tests in Jest. - -Assume nobody cares _how_ your code works, we just need to know that the public -API you designed _does_ work. If performance problems or new tech necessitate a -re-write, these tests become a guide for the next implementation. - -#### Wrap native browser resources in a function so they can be easily mocked - -If your javascript needs to talk to the window object, or navigator, etc. wrap -that in a function call so that it can be easily mocked during testing. - -```javascript -// myModule.js - -// ... other code - -export function redirectTo(url) { - window.location = url; -} - -// myModule.test.js - -jest.mock("myModule", () => ({ - redirectTo: (url) => { - console.log(`I would have gone to: ${url}`); - }, -})); - -describe("some module", () => { - // .... - it("Performs a redirect when Foo is clicked"){ - expect(redirectTo.mock.calls.length).toBe(1); - } -}); -``` - -#### Implement logic in pure functions when possible - -The more of your logic that is written in deterministic functions (i.e. no -side-effects, same inputs always result in same outputs) the easier it is to -test. Just load up the functions and supply suitable test inputs. - -There is almost definitely no such thing as a well-written 1000 line function. -Most of whatever happened in that thing was probably deterministic and can be -broken up into easily testable chunks. - -### Specific test scenarios & examples - -[Mocking an imported -dependency](https://github.com/galaxyproject/galaxy/blob/dev/client/src/components/Tags/tagService.test.js) - -[Testing async -operations](https://github.com/galaxyproject/galaxy/blob/dev/client/src/components/Tags/tagService.test.js) - -[Testing a Vue component for expected rendering -output](https://github.com/galaxyproject/galaxy/blob/dev/client/src/components/Tags/StatelessTags.test.js) - -[Firing an event against a shallow mounted vue -component](https://github.com/galaxyproject/galaxy/blob/dev/client/src/components/Tags/StatelessTags.test.js) - -### The dirty secret about testing - -It's good to have tests, but testing code isn't really about testing at all, -it's actually about software design. - -Maintainable software is testable software. If you can run a unit test on your -code, then that means you must have necessarily separated your code into -testable units and you will have almost definitely written better, more -modular, more easily manipulated code, and nobody will ever contemplate using -git-blame to figure out what went wrong. - -Probably. diff --git a/client/docs/src/component-design/avoid-using-Galaxy.md b/client/docs/src/component-design/avoid-using-Galaxy.md deleted file mode 100644 index 15b38adc5d2..00000000000 --- a/client/docs/src/component-design/avoid-using-Galaxy.md +++ /dev/null @@ -1,14 +0,0 @@ -Don't directly reference window.Galaxy in Vue components - -You've got components. Components take props. Please pass in any values you might need from -window.Galaxy as props and avoid referencing global Galaxy inside your components. I've even created -basic providers which give you access to the Galaxy.config, current user, and current user -histories. Please use them to retrieve your values, and bypass importing Galaxy altogether. - -There are use-cases where the Backbone models update over time and we need to update some value -inside Vue. Let me help you solve those problems instead of importing backbone models into Vue -components. Usually the answer is a backbone event listener that updates some relevant Vuex store. - -When writing a new component, your goal should be to replace old Galaxy functionality, not to -repackage it in another format so we continue to have Galaxy's inherent problems, the most important -of which is... \ No newline at end of file diff --git a/client/docs/src/component-design/avoid-using-global-Galaxy.md b/client/docs/src/component-design/avoid-using-global-Galaxy.md new file mode 100644 index 00000000000..4a65dd9f429 --- /dev/null +++ b/client/docs/src/component-design/avoid-using-global-Galaxy.md @@ -0,0 +1,77 @@ +Without a doubt. The most problematic part of the old client is the way every important piece of +information was hung on a global variable whose initialization is completely unregulated. + +### Don't directly reference window.Galaxy in Vue components + +Components take props. Please pass in any values you might need from window.Galaxy as props and +avoid referencing global Galaxy inside your components. I've even created basic providers which give +you access to the Galaxy.config, current user, and current user histories. Please use them to +retrieve your values, and bypass importing Galaxy altogether. + +#### Sometimes you still need to update Vue from backbone as the legacy environment changes +There are definitely use-cases where the Backbone models update over time and we need to update some +value inside Vue. Let me help you solve those problems instead of importing backbone models into Vue +components. Usually the answer is a backbone event listener that updates some relevant Vuex store. + +* [Keeping Vuex in Sync with + Galaxy](https://github.com/galaxyproject/galaxy/blob/dev/client/src/store/syncVuexToGalaxy.js) + +These issues should disappear over time as the all of the old client is rebuilt in the new +ecosystem. + + +## Mount Functions + +When writing a new component, your goal should be to replace old Galaxy functionality, not to +repackage it in another format thereby persisting Galaxy's inherent problems. + +In what most people think of as a "standard" Vue application there would be only one place that Vue +is mounted to the HTML environment, and that would be in a main.j or an app.js. Most modern +single-page applications only have one starting point like that. + +However, we are incrementally replacing old Backbone views, so in its current state, Galaxy may have +several mounting functions for various components depending on where that component is intended to +fit into the existing Backbone layouts. + + +### Use the standard mount function + +A standard mount function has been provided in src/utils. This mount function accepts a component +definition, and then allows you to start up your Vue component with our standard load-out of plugins +including localization, Vuex, and a couple other utilities. This is the preferred way to mount your +component inside the old Backbone layout until the application is fully converted. + +```js static +// mountMyComponent.js + +import { mountVueComponent } from "utils/mountVueComponent"; +import MyComponent from "./MyComponent"; + +// this creates a function that will accept a props object and a DOM element on which to mount +const mounter = mountVueComponent(MyComponent); +export default mounter; +``` + +### Using the standard mount to pass in Galaxy variables as props + +```js static +import { getGalaxyInstance } from "app"; +import mountMyComponent from "./mountMyComponent"; + +const HorribleBackboneView = { + + someInitMethodYouMake() { + const Galaxy = getGalaxyInstance(); + + // pass in required props + const props = { + somePropVal: Galaxy.someDealie, + name: Galaxy.currentHistory.name, + }; + + // VM is a Vue instance. + // this.$el is some jquery selection, first item is the actual DOM object + const vm = mountMyComponent(props, this.$el[0]); + } +} +``` diff --git a/client/docs/src/component-design/javascript-noob.md b/client/docs/src/component-design/javascript-noob.md deleted file mode 100644 index 6da166521a7..00000000000 --- a/client/docs/src/component-design/javascript-noob.md +++ /dev/null @@ -1,14 +0,0 @@ -### Classes exist but they aren't as important as you may be used to. - -You may come from a class-based programming background and think that your first step should be to -make a class hierchy that does the thing you want. It doesn't help that Vue superficially looks like -a class definition, (even though what you're really doing is configuring an Observable tree). - -### Javascript is a pretty functional language. - -Javascript is largely a functional language. Its class support is limited and less useful, and -unless we switch to Typescript, we don't even have interfaces or typing, arguably the most useful -parts of a class-based language. - -In general, you will get more mileage out of javascript by embracing functional programming -approaches because that is what Javascript is good at. diff --git a/client/docs/src/component-design/never-use-jquery.md b/client/docs/src/component-design/never-use-jquery.md index 1c89e6b9dd3..c1e07181928 100644 --- a/client/docs/src/component-design/never-use-jquery.md +++ b/client/docs/src/component-design/never-use-jquery.md @@ -11,4 +11,8 @@ site whether you want it or not. jQuery leverages an outdated initialization par hugely problematic when it comes to unit testing and module building. One of our most important goals in redesigning Galaxy is the complete elimination of this library -from our source, along with all its invasive plugins. \ No newline at end of file +from our source, along with all its invasive plugins. + +### References +* [You Don't Need jQuery](https://github.com/nefe/You-Dont-Need-jQuery) +* [document.querySelector](https://developer.mozilla.org/en-US/docs/Web/API/Document/querySelector) \ No newline at end of file diff --git a/client/docs/src/component-design/providers-and-renderers.md b/client/docs/src/component-design/providers-and-renderers.md index ed4064dfe6f..03fc612fefe 100644 --- a/client/docs/src/component-design/providers-and-renderers.md +++ b/client/docs/src/component-design/providers-and-renderers.md @@ -3,7 +3,7 @@ probably look familiar to anybody whis is already passingly familiar with Vue. H information comes in as properties, any internal variables get defined in "data", changes go out as events. -## Composition Example +### Composition Example ```html static diff --git a/client/docs/src/component-design/readme.md b/client/docs/src/component-design/readme.md new file mode 100644 index 00000000000..dba0f97c1e1 --- /dev/null +++ b/client/docs/src/component-design/readme.md @@ -0,0 +1,71 @@ +### A component is really just a fancy function + +I'm not talking about how webpack turns it into a rendering function. That's obvious. + +I mean conceptually, props come in (like arguments) and events go out (like the return statements). +A component is a fancy kind of function that can keep emitting results and accept changing inputs +over time. In truth it more closely resembles an Observable, but an observable is ALSO a slightly +fancier kind of function. + +If you just think of a component as thing that takes input props and emits output events you're well +on your way to using them well. + +The worst components are ones that might as well just be a big single page script. That's zero +percent better than the spaghetti we're working so hard to replace. That's just repackaging all the +problems of the old imperative class-based legacy code. + +### Get comfortable with events, limit your dependence on Vuex + +New vue programmers are ok at handing props to components, but they rarely use events effectively +(at first). As a result they end up using a lot of global state, a million little data props and +relying on imperfect globalized tools like Vuex or other imported dependencies for every little +variable. + +Vuex definitely has its uses, but not as many as you might expect given the way it is +overly-emphasized in common tutorials. It's easy to walk away from an "Intro to Vue" video with the +idea that all data must live in Vuex all the time. That's a really undesirable situation. + +Although vuex is a well-organized (many would say over-organized) state machine, it is important +to remember that it is still a kind of global injection and deserves to be considered as such. + +* [Should I Store This Data in + Vuex](https://markus.oberlehner.net/blog/should-i-store-this-data-in-vuex/) +* [Vuex getters are great, but don’t overuse + them](https://codeburst.io/vuex-getters-are-great-but-dont-overuse-them-9c946689b414) + +Data persistence should be something that happens near the top of your component tree, not down in +the guts. + +Your first thought with a component should be: "How can I offload the handling of the results of +this component to my caller?" The answer is usually going to be events. A component that simply +accepts props and emits events can be re-used in more contexts than one that relies on an external +global state to operate. + +#### Read up on .sync and v-model + +They're just fancy shorthands for a prop / event handler combination. They are fundamentally no +different from props and events, but the syntax is important to understand. + + +### Think carefully about what should really be in "data". + +Most of good component design boils down to answering the following question: What do I want to put +in data, computed, and props? + +Data is the place where temp data goes that is not sensible to persist in Vuex or other +application-wide global state, usually because its use is very specific to the operation of this +particular component. There really should only be a few variables in data. + +#### Break down your own internal dependencies + +Most components only need one or two variables in data. If you have a large amount of data +variables, it's worth taking a little time to stop and build a mini-dependency tree for yourself. +You will probably find that almost everything can be written in terms of computed transformations on +a small number of data and properties. + +Defining your values using computes will lead to a lot less "oh I forgot to update that +variable"-style bugs. + +If you have more than a few data variables you probably (a) aren't leveraging computeds and +properties, or (b) are trying to implement too many features for just one component. It is important +separate your concerns in components just like you do in any other kind of programming. diff --git a/client/docs/src/component-design/testing-advice.md b/client/docs/src/component-design/testing-advice.md deleted file mode 100644 index 8f9d1f2e08b..00000000000 --- a/client/docs/src/component-design/testing-advice.md +++ /dev/null @@ -1,3 +0,0 @@ -## Wrap access to browser-native resources in functions - -Unit testing in a modern javascript application runs in node without a browser. \ No newline at end of file diff --git a/client/docs/src/component-design/unit-testing/readme.md b/client/docs/src/component-design/unit-testing/readme.md new file mode 100644 index 00000000000..393c1a0c117 --- /dev/null +++ b/client/docs/src/component-design/unit-testing/readme.md @@ -0,0 +1,20 @@ + +[Galaxy uses Jest](https://jestjs.io/) for its client-side unit testing +framework. + +For testing Vue components, we use the [Vue testing +utils](https://vue-test-utils.vuejs.org/) to mount individual components in a +test bed and check them for rendered features. Please use jest-based mocking +for isolating test functionality. + + +### Specific test scenarios & examples + +* [Mocking an imported +dependency](https://github.com/galaxyproject/galaxy/blob/dev/client/src/components/Tags/tagService.test.js) +* [Testing async +operations](https://github.com/galaxyproject/galaxy/blob/dev/client/src/components/Tags/tagService.test.js) +* [Testing a Vue component for expected rendering +output](https://github.com/galaxyproject/galaxy/blob/dev/client/src/components/Tags/StatelessTags.test.js) +* [Firing an event against a shallow mounted vue +component](https://github.com/galaxyproject/galaxy/blob/dev/client/src/components/Tags/StatelessTags.test.js) diff --git a/client/docs/src/component-design/unit-testing/strategies.md b/client/docs/src/component-design/unit-testing/strategies.md new file mode 100644 index 00000000000..f2c60c24783 --- /dev/null +++ b/client/docs/src/component-design/unit-testing/strategies.md @@ -0,0 +1,42 @@ +Part of making good code is making that code easy to test. + +### Implement logic in pure functions when possible + +The more of your logic that is written in deterministic functions (i.e. no +side-effects, same inputs always result in same outputs) the easier it is to +test. Just load up the functions and supply suitable test inputs. + +There is almost definitely no such thing as a well-written 1000 line function. +Most of whatever happened in that thing was probably deterministic and can be +broken up into easily testable chunks. + + +### Wrap native browser resources in a function so they can be easily mocked + +If your javascript needs to talk to the window object, or navigator, etc. wrap +that in a function call so that it can be easily mocked during testing. + +```js static +// myModule.js + +// ... other code + +export function redirectTo(url) { + window.location = url; +} + +// myModule.test.js + +jest.mock("myModule", () => ({ + redirectTo: (url) => { + console.log(`I would have gone to: ${url}`); + }, +})); + +describe("some module", () => { + // .... + it("Performs a redirect when Foo is clicked"){ + expect(redirectTo.mock.calls.length).toBe(1); + } +}); +``` \ No newline at end of file diff --git a/client/docs/src/component-design/unit-testing/writing-tests.md b/client/docs/src/component-design/unit-testing/writing-tests.md new file mode 100644 index 00000000000..8951b5797de --- /dev/null +++ b/client/docs/src/component-design/unit-testing/writing-tests.md @@ -0,0 +1,73 @@ +### Clearly document the intent of your test + +Please remember that these tests are not _for_ you. They're for the people who +come after you. It will be a lot easier to modify, repair and upgrade your code +if they can figure out what you were originally hoping to accomplish. Try to +use as detailed 'expect' statements as possible -- overuse of 'toBeTruthy()' +for example, can hide the intent of your test. + +Add a couple of comments. Use variable names that mean something. Nobody's +code is as self-documenting as they believe it to be. + + +### Only test the public API that you define + +Internal implementations come and go with library upgrades and new tech. But +the point of the unit test is to make sure your units work as designed.... +which means you need to... you know... design your code to work in units. + +Separate your concerns and identify the developer-facing methods and functions +you expect them to use. Test THOSE. Everything else should probably be +considered an implementation detail. + +The other side of the same coin is to test *only* the unit in question. If your +component has a model that uses a service that touches Vuex, which then uses +Axios to fetch some data -- don't test all that at once. Break things apart and +mock functionality to isolate testing to units. End to end testing is a +separate thing that shouldn't be attempted using spec tests in Jest. + +Assume nobody cares _how_ your code works, we just need to know that the public +API you designed _does_ work. If performance problems or new tech necessitate a +re-write, these tests become a guide for the next implementation. + + +### Writing a test file + +Jest will try to test any file ending in "\*.test.js". Please place your test +files inside client/src folders right next to whatever files that they are +testing. + +Jest has extensive documentation on the expect API, mocking, and more on the +[official docs page](https://jestjs.io/docs/en/getting-started.html), which will +be your best resource here. + +```js static +// yourcode.test.js + +import { things } from "./yourcode.js"; + +describe("some module you wrote", () => { + let transientVariables; + let serviceInstances; + let testData; + + beforeEach(() => { + // setup your code (if necessary) + }); + + afterEach(() => { + // teardown your code (so it doesn't ruin the next test) + }); + + it("should do something or other", () => { + expect(workflowNodeCount()).toBe(5); + }); +}); +``` + + +### Check out the Jest helper functions + +We have created some [common helpers for common testing +scenarios](https://github.com/galaxyproject/galaxy/blob/dev/client/tests/jest/helpers.js). + diff --git a/client/docs/src/component-design/vue-component-tips.md b/client/docs/src/component-design/vue-component-tips.md deleted file mode 100644 index c90ff309aaf..00000000000 --- a/client/docs/src/component-design/vue-component-tips.md +++ /dev/null @@ -1,82 +0,0 @@ -## Understand that a component is really just a fancy function - -I'm not talking about how webpack turns it into a rendering function. That's obvious. - -I mean conceptually, props come in (like arguments) and events go out (like the return statements). -A component is a fancy kind of function that can keep emitting results and accept changing inputs -over time. In truth it more closely resembles an Observable, but an observable is ALSO a slightly -fancier kind of function. - -If you just think of a component as thing that takes inputs and emits outputs you're well on your -way to using them properly. - -The worst components are ones that might as well just be a big single script that runs. That's zero -percent better than the spaghetti we're working so hard to replace. That's just repackaging all the -problems of the old imperative class-based legacy code. - - -## Javascript is a pretty functional language. Classes exist but they aren't as important as you may be used to. - -You may come from a class-based programming background and think that your first step should be to -make a class hierchy that does the thing you want. It doesn't help that Vue superficially looks like -a class definition, (even though what you're really doing is configuring an Observable tree). - -But javascript is largely a functional language. Its class support is limited and less useful, and -unless we switch to Typescript, we don't even have interfaces or typing, arguably the most useful -parts of a class-based language. - -In general, you will get more mileage out of javascript by embracing functional programming -approaches because that is what Javascript is good at. - - -## Get comfortable with events, limit your dependence on Vuex - -New vue programmers are ok at handing props to components, but they rarely use events effectively -(at first). As a result they end up using a lot of global state, a million little data props and -relying on imperfect globalized tools like Vuex or other imported dependencies for every little -variable. - -Vuex has its uses, but not as many as you might think. Data persistence should be something that -happens near the top of your component tree, not down in the guts. - -Your first thought with a component should be: "How can I offload the handling of the results of -this component to my caller?" The answer is usually going to be events. A component that simply -accepts props and emits events can be re-used in more contexts than one that relies on an external -global state to operate. - -### Read up on .sync and v-model - -They're just fancy shorthands for a prop / event handler combination. They are fundamentally no -different from props and events, but the syntax is important to understand. - - - -## Think carefully about what should really be in "data". - -Most of good component design boils down to answering the following question: What do I want to put -in data, computed, and props? - -Data is the place where temp data goes that is not sensible to persist in Vuex or other -application-wide global state, usually because its use is very specific to the operation of this -particular component. There really should only be a few variables in data. - -### Break down your own internal dependencies - -Most components only need one or two variables in data. If you take the time to analyze your own -internal dependency tree, you will probably find that almost everything can be written in terms of -computed transformations on a small number of data and propertie - -If you have more than a few variables you (a) aren't leveraging computeds and properties, or (b) are -trying to implement too many features for just one component. It is important separate your concerns -in components just like you do in any other kind of programming. - - - -## Have a plan - -Create a design plan and an abstraction for the way the guts of your component work. Think about the -way data passes from parent components to children and back again. Break your component into -sub-components just like you would break a class into sub-methods, the same exact principles apply. - -Don't just dump a pile of spaghetti into a component, that is no better than the legacy code we are -replacing. \ No newline at end of file diff --git a/client/styleguide.config.js b/client/styleguide.config.js index 3ecd66dd6c9..b2cc06d89bd 100644 --- a/client/styleguide.config.js +++ b/client/styleguide.config.js @@ -34,7 +34,7 @@ function getSections() { const cmpPath = path.join(__dirname, "src/components"); const { rootNode: componentDocs } = getDocSections(cmpPath, { ignore: problemChildren }); - return [design, componentDocs, styles]; + return [design, styles, componentDocs]; } module.exports = {