Update to Storybook v6, add base TypeScript support at project level and add typings for two packages #250

evansiroky · 2021-05-16T07:45:21Z

This is the first of many PRs to add TypeScript types throughout this project. This first PR sets up the overall project to be able to handle TypeScript in the first place. As a part of adding TypeScript, Storybook was also updated to v6 to be able to take advantage of the automatic generation of Storybook documentation from type definitions.

This PR does the following specifically:

Project-wide changes

Typescript stuff

Adds the TypeScript preset to the babel config so that babel can still transpile TypeScript files
Adds a TypeScript script that will run TypeScript within all packages where the TypeScript script is defined. This will also result in the generation of type declaration files that can be included in releases so that other packages can read in these typed files
Changes the prepublish script to also run TypeScript which will generate TypeScript typings for each project configured to have typings
Adds TypeScript support within the linting process by change the base linting configuration from airbnb to airbnb-typescript, adding the @typescript-eslint/recommended plugin, and substituting a few common eslint checks for TypeScript-friendly checks. (see more info on this as it might come up later).
Makes sure stylelint works with .ts files (but doesn't analyze generated *.d.ts files).

Storybook stuff

Updates all packages and addons to v6 (also replaces addon-info with addon-docs) (fixes Storybook: look to switch from addon-info to addon-docs #68, fixes Storybook: upgrade to v6 #187).
Updates a few babel plugins so they play nicely with Storybook (all the entries that have options of { loose: true }).
Refactor all storiesOf stories into CSF format.

Changes to base-map package

Added a few eslint-disables to some new eslint typescript rules. We might deal with these later if needed when adding TypeScript support for this package.
Moved story info text to component class JSDoc

Changes to humanize-duration package

Adds complete TypeScript coverage to the package
Adds a TypeScript test file to the package to prove that writing tests in TypeScript works and is executed properly by jest
Adds TypeScript type path, script and configuration file to make sure TypeScript type declarations are generated and (🤞) included in future releases

Changes to icons package

Refactors stories to be more meaningful. Instead of showing each icon as a different story, stories are now shown for the various LegIcon and ModeIcon components.

Changes to itinerary-body package

Refactors stories so that they are separated into otp-ui and otp-react-redux folders.

Changes to location-field package

Changed around the order of some variable declarations to fix a linting error.
Refactors stories so that they are separated into "Desktop Context" and "Mobile Context" folders.

Changes to location-icon package

Adds complete TypeScript coverage to the package
Adds TypeScript type path, script and configuration file to make sure TypeScript type declarations are generated and (🤞) included in future releases

landonreed · 2021-05-17T15:01:17Z

Storybook 6 should hopefully parse TypeScript typings and JSDoc out-of-the-box

@evansiroky, let's just bump to Storybook 6 in that case? Unless the migration is overly cumbersome?

binh-dam-ibigroup · 2021-05-18T20:22:34Z

packages/location-icon/src/LocationIcon.story.tsx

@@ -1,4 +1,4 @@
-import React from "react";


This story is not being shown in Storybook. Add the .story.tsx extension to /.storybook/config.js.

binh-dam-ibigroup

Upgrading storybook doesn't look straight forward and will need some more investigation.

fpurcell

If no objections, I'm going to merge in #242 and then deploy Storybook of latest 'dev' to the github pages site.

evansiroky · 2021-05-27T07:07:09Z

Ready for review again, this time with an update to Storybook v6.

packages/humanize-distance/src/index.ts

landonreed · 2021-05-27T17:38:25Z

packages/icons/src/stories/classic-leg-icon.story.js

@@ -0,0 +1,14 @@
+import React from "react";


Is there not a way to output the leg- and mode-icon stories as they previously were handled? This new code just seems like a lot of extra files/boilerplate.

I agree to the extent that some of the related code is a bit overkill and not directly related to the PR, although I like the grouping by each LegIcon and ModeIcon component.

There is not a way to output as they were before since there is no way to dynamically generate a lot of stories once storiesOf is deprecated. See storybookjs/storybook#9828. The previous way was also quite difficult to browse and somewhat meaningless. This new method shows the output of the various LegIcon and ModeIcon components which I feel is an improvement.

landonreed · 2021-05-27T17:41:24Z

packages/location-field/src/styled.js

+export const MenuItemA = styled.a`
+  background-color: ${props => (props.active ? "#337ab7" : "transparent")};
+  clear: both;
+  color: ${props => (props.active ? "#fff" : "#333")};
+  display: block;
+  font-weight: 400;
+  line-height: 1.42857143;
+  padding: 3px 20px;
+  text-decoration: none;
+  white-space: nowrap;
+`;
+
+export const MenuItemHeader = styled.li`
+  color: navy;
+  display: block;
+  font-size: 14px;
+  font-weight: 600;
+  line-height: 1.42857143;
+  padding: 3px 20px;
+  text-align: ${props => (props.centeredText ? "center" : "left")};
+  white-space: nowrap;
+`;
+
+export const MenuItemLi = styled.li`
+  &:hover {
+    background-color: ${props => !props.disabled && "#f5f5f5"};
+  }
+`;
+


Are these components just moved around this file? If so, I don't fully understand why?

As part of updating the eslint config, the new rule of no-use-before-define became active. The moving around of these variables is to comply with this new rule.

binh-dam-ibigroup

Please add to the PR description about a fix to #187. Consider some of the minor tweaks mentioned. My comments regarding LegIcon (#27) are non-blocking, it might be worth an internal discussion.

binh-dam-ibigroup · 2021-06-01T14:00:26Z

.storybook/manager.js

+
+addons.setConfig({
+  sidebar: {
+    showRoots: false


What does this setting do? (probably needs a comment if we are overriding a default)

Added a comment in 2695404

binh-dam-ibigroup · 2021-06-01T14:10:19Z

babel.config.js

    "babel-plugin-styled-components"
  ],
  presets: [
    ["@babel/preset-env", { targets: { node: "current" } }],
+    ["@babel/preset-typescript"],


Remove array notation if we are not passing extra params.

Refactored in 2695404

binh-dam-ibigroup · 2021-06-01T14:40:30Z

packages/icons/src/stories/classic-leg-icon.story.js

@@ -0,0 +1,14 @@
+import React from "react";


I agree to the extent that some of the related code is a bit overkill and not directly related to the PR, although I like the grouping by each LegIcon and ModeIcon component.

binh-dam-ibigroup · 2021-06-01T14:46:50Z

packages/icons/src/stories/leg-icon-renderer.js

+const bikeOnlyItinerary = require("@opentripplanner/itinerary-body/src/__mocks__/itineraries/bike-only.json");
+const bikeRentalItinerary = require("@opentripplanner/itinerary-body/src/__mocks__/itineraries/bike-rental.json");
+const eScooterRentalItinerary = require("@opentripplanner/itinerary-body/src/__mocks__/itineraries/e-scooter-rental.json");
+const parkAndRideItinerary = require("@opentripplanner/itinerary-body/src/__mocks__/itineraries/park-and-ride.json");
+const tncTransitTncItinerary = require("@opentripplanner/itinerary-body/src/__mocks__/itineraries/tnc-transit-tnc.json");
+const walkOnlyItinerary = require("@opentripplanner/itinerary-body/src/__mocks__/itineraries/walk-only.json");
+const walkTransitWalkItinerary = require("@opentripplanner/itinerary-body/src/__mocks__/itineraries/walk-transit-walk.json");
+const walkTransitWalkTransitWalkItinerary = require("@opentripplanner/itinerary-body/src/__mocks__/itineraries/walk-transit-walk-transit-walk.json");
+
+const exampleLegs = [
+  { leg: bikeOnlyItinerary.legs[0], type: "Personal bike" },
+  { leg: bikeRentalItinerary.legs[1], type: "Bike rental" },
+  { leg: walkTransitWalkTransitWalkItinerary.legs[1], type: "Bus" },
+  {
+    leg: eScooterRentalItinerary.legs[1],
+    type: "Micromobility (from Razor company)"
+  },
+  { leg: parkAndRideItinerary.legs[0], type: "Park & Ride" },
+  { leg: tncTransitTncItinerary.legs[0], type: "TNC (Uber)" },
+  { leg: walkTransitWalkItinerary.legs[1], type: "Tram" },
+  { leg: walkOnlyItinerary.legs[0], type: "Walk" }
+];


Importing an entire leg seems overkill. But maybe that comes from the redundancy between ModeIcon and LegIcon, so should we address #27 in the near future?

The leg is needed because it is an input into a LegIcon component.

I'm not sure how this related to #27. I believe we may have already resolved it by have a TriMetLegIcon, ClassicLegIcon and StandardLegIcon classes.

Might be for another discussion... if we addressed #27 you wouldn't have to load all of these itineraries just to extract one leg and ultimately the mode for that leg.

packages/humanize-distance/src/index.ts

packages/location-icon/src/index.tsx

evansiroky added 2 commits May 16, 2021 00:08

feat: add typescript support for two packages

f689783

feat: add types to location-icon package.json

d15c1a5

evansiroky assigned fpurcell, landonreed and binh-dam-ibigroup May 16, 2021

landonreed assigned evansiroky May 17, 2021

binh-dam-ibigroup reviewed May 18, 2021

View reviewed changes

fpurcell reviewed May 18, 2021

View reviewed changes

evansiroky added 3 commits May 24, 2021 22:28

Merge branch 'otp/master' into typescript

77eb527

chore: update Storybook from v5 to v6

423c8fa

test: turn off some eslint rules conflicting with prettier

01b7308

evansiroky changed the title ~~Add base TypeScript support at project level and add typings for two packages~~ Update to Storybook v6, add base TypeScript support at project level and add typings for two packages May 27, 2021

evansiroky removed their assignment May 27, 2021

landonreed reviewed May 27, 2021

View reviewed changes

packages/humanize-distance/src/index.ts Show resolved Hide resolved

landonreed reviewed May 27, 2021

View reviewed changes

binh-dam-ibigroup approved these changes Jun 1, 2021

View reviewed changes

binh-dam-ibigroup assigned evansiroky and unassigned binh-dam-ibigroup Jun 1, 2021

refactor: add some comments

2695404

evansiroky removed their assignment Jun 5, 2021

docs: properly encapsulate table headers in thead tag

834ca39

evansiroky mentioned this pull request Jun 9, 2021

Produce esm modules during prepublish scripts #256

Merged

landonreed approved these changes Jun 14, 2021

View reviewed changes

landonreed assigned evansiroky and unassigned landonreed Jun 14, 2021

fpurcell approved these changes Jun 15, 2021

View reviewed changes

evansiroky merged commit ef6aa0a into opentripplanner:master Jun 18, 2021

evansiroky deleted the typescript branch June 18, 2021 04:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to Storybook v6, add base TypeScript support at project level and add typings for two packages #250

Update to Storybook v6, add base TypeScript support at project level and add typings for two packages #250

evansiroky commented May 16, 2021 •

edited

Loading

landonreed commented May 17, 2021 •

edited

Loading

binh-dam-ibigroup May 18, 2021

binh-dam-ibigroup left a comment

fpurcell left a comment •

edited

Loading

evansiroky commented May 27, 2021

landonreed May 27, 2021

binh-dam-ibigroup Jun 1, 2021

evansiroky Jun 5, 2021

landonreed May 27, 2021

evansiroky Jun 5, 2021

binh-dam-ibigroup left a comment

binh-dam-ibigroup Jun 1, 2021

evansiroky Jun 5, 2021

binh-dam-ibigroup Jun 1, 2021

evansiroky Jun 5, 2021

binh-dam-ibigroup Jun 1, 2021

binh-dam-ibigroup Jun 1, 2021

evansiroky Jun 5, 2021

binh-dam-ibigroup Jun 8, 2021

Update to Storybook v6, add base TypeScript support at project level and add typings for two packages #250

Update to Storybook v6, add base TypeScript support at project level and add typings for two packages #250

Conversation

evansiroky commented May 16, 2021 • edited Loading

Project-wide changes

Typescript stuff

Storybook stuff

Changes to base-map package

Changes to humanize-duration package

Changes to icons package

Changes to itinerary-body package

Changes to location-field package

Changes to location-icon package

landonreed commented May 17, 2021 • edited Loading

Choose a reason for hiding this comment

binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

fpurcell left a comment • edited Loading

Choose a reason for hiding this comment

evansiroky commented May 27, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evansiroky commented May 16, 2021 •

edited

Loading

landonreed commented May 17, 2021 •

edited

Loading

fpurcell left a comment •

edited

Loading