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 Storybook #8

Merged
merged 13 commits into from
Jul 6, 2021
Merged

add Storybook #8

merged 13 commits into from
Jul 6, 2021

Conversation

PiDelport
Copy link
Collaborator

@PiDelport PiDelport commented Jun 29, 2021

@PiDelport PiDelport added the M: web-client Module: Web client app (Angular) label Jun 29, 2021
@codecov
Copy link

codecov bot commented Jun 29, 2021

Codecov Report

Merging #8 (f92eca3) into web-client/build-add-prettier (0eec038) will not change coverage.
The diff coverage is n/a.

❗ Current head f92eca3 differs from pull request most recent head ac7bc77. Consider uploading reports for the commit ac7bc77 to get more accurate results
Impacted file tree graph

@@                       Coverage Diff                       @@
##           web-client/build-add-prettier        #8   +/-   ##
===============================================================
  Coverage                         100.00%   100.00%           
===============================================================
  Files                                  4         4           
  Lines                                  8         8           
===============================================================
  Hits                                   8         8           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0eec038...ac7bc77. Read the comment docs.

@PiDelport PiDelport mentioned this pull request Jun 29, 2021
5 tasks
@PiDelport PiDelport marked this pull request as draft June 29, 2021 15:45
@PiDelport PiDelport changed the base branch from main to web-client/build-add-prettier June 29, 2021 15:45
@PiDelport PiDelport force-pushed the add-storybook branch 2 times, most recently from 6d77d9c to 3760107 Compare June 29, 2021 22:59
@PiDelport PiDelport added this to In progress in Nautilus Wallet via automation Jul 1, 2021
@PiDelport
Copy link
Collaborator Author

PiDelport commented Jul 1, 2021

Hmm, it looks like one of the Storybook dependencies in 9e7dc54 breaks build:prod (but not build).

I'll investigate.

@PiDelport PiDelport mentioned this pull request Jul 1, 2021
2 tasks
@PiDelport
Copy link
Collaborator Author

Investigation

It looks like this is one those npm issues… 🤦🏼‍♀️

At first, I had trouble reproducing this reliably: a normal npm install would not trigger the crash after updating to the breaking changeset (or stop the crash, after switching to the parent changeset), but a full npm ci would.

In theory, the result of those two commands should be the same, but they're not.

To inspect closer, 🕵🏼‍♀️ I compared snapshots of the working and non-working node-modules, and apparently the breaking difference is that the contents of the cssnano-utils package somehow gets replaced with the contents of the postcss-values-parser package (which doesn't seem related), in the same directory. 🤯

It looks like something went wrong in package-lock.json:

     "cssnano-utils": {
       "version": "2.0.1",
-      "resolved": "https://registry.npmjs.org/cssnano-utils/-/cssnano-utils-2.0.1.tgz",
-      "integrity": "sha512-i8vLRZTnEH9ubIyfdZCAdIdgnHAUeQeByEeQ2I7oTilvP9oHO6RScpeq3GsFUVqeB8uZgOQ9pw8utofNn32hhQ==",
+      "resolved": "https://registry.npmjs.org/postcss-values-parser/-/postcss-values-parser-2.0.1.tgz",
+      "integrity": "sha512-2tLuBsA6P4rYTNKCXYG/71C7j1pU6pK503suYOmn4xYrQIzW+opD+7FAFNuGSdZC/3Qfy334QbeMu7MEb8gOxg==",
       "dev": true,
-      "requires": {}
+      "requires": {
+        "flatten": "^1.0.2",
+        "indexes-of": "^1.0.1",
+        "uniq": "^1.0.1"
+      }
     },

This could have happened due to some npm resolution weirdness, or a particularly unlucky automatic mis-merge alignment, or something else?

I'll try to fix this by replaying the dependency additions in a separate commit.

@PiDelport
Copy link
Collaborator Author

That fixed it! 🎉

Diff: 3760107..bb95236

@PiDelport PiDelport force-pushed the add-storybook branch 3 times, most recently from c9e9a7b to ddf3989 Compare July 1, 2021 18:26
@PiDelport PiDelport moved this from In progress to Review in progress in Nautilus Wallet Jul 1, 2021
@PiDelport PiDelport added the CI/CD label Jul 1, 2021
@jongbonga jongbonga marked this pull request as ready for review July 2, 2021 12:50
Nautilus Wallet automation moved this from Review in progress to Reviewer approved Jul 2, 2021
Steps:

1. `npx sb init`
2. Fix @angular/elements version to ~12.0.1 (to match the others)
3. `npm run format`
4. Undo tsconfig.app.json comment removal
5. Skip committing the default example stories
This avoids pulling in the generated "www" directory, among others.
This avoids leaving a documentation.json in the root.
This actually lets Storybook stories type-check more easily.
npm install --save-dev chromatic
…12.0.4+

@angular-devkit/build-angular 12.0.4+ makes build-storybook hang.

Constraining it to 12.0.3 avoids the hang.

Upstream issue, and workaround:

storybookjs/storybook#15227 (comment)

Also jump through some hoops to downgrade webpack from 5.39.1 to 5.38.1,
to match the exact dependency expected by @angular-devkit/build-angular,
and avoid this problem:

angular/angular-cli#20773
@PiDelport PiDelport changed the base branch from web-client/build-add-prettier to main July 6, 2021 07:23
@PiDelport PiDelport merged commit 74fec6e into main Jul 6, 2021
Nautilus Wallet automation moved this from Reviewer approved to Done Jul 6, 2021
@PiDelport PiDelport assigned PiDelport and jongbonga and unassigned PiDelport and jongbonga Jul 15, 2021
@PiDelport PiDelport deleted the add-storybook branch July 23, 2021 12:18
@PiDelport PiDelport removed this from the Milestone 1: Tech MVP milestone Sep 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/CD M: web-client Module: Web client app (Angular)
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants