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

docs(misc): change http:// to https:// #19534

Merged
merged 1 commit into from Jan 30, 2024
Merged

Conversation

beeman
Copy link
Contributor

@beeman beeman commented Oct 10, 2023

Current Behavior

I found a few typos (and http:// that could be https://) while looking through the Nx source code.

Expected Behavior

It has fewer typos.

@beeman beeman requested a review from a team as a code owner October 10, 2023 15:18
@vercel
Copy link

vercel bot commented Oct 10, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nx-dev ✅ Ready (Inspect) Visit Preview Jan 30, 2024 4:27pm

@beeman beeman requested review from a team and Coly010 as code owners January 30, 2024 16:21
@mandarini mandarini changed the title chore(repo): fix typos docs(misc): change http:// to https:// Jan 30, 2024
@AgentEnder AgentEnder merged commit a9974d3 into nrwl:master Jan 30, 2024
4 of 6 checks passed
@beeman beeman deleted the beeman/typos-etc branch January 30, 2024 17:47
FrozenPandaz pushed a commit that referenced this pull request Jan 31, 2024
@marleypowell
Copy link

This appears to have broken some stuff when running storybook commands

Error: no schema with key or ref "https://json-schema.org/schema"
    at Ajv.validate (C:\repo\node_modules\ajv\lib\core.ts:359:21)
    at Ajv.validateSchema (C:\repo\node_modules\ajv\lib\core.ts:515:24)
    at Ajv._addSchema (C:\repo\node_modules\ajv\lib\core.ts:721:30)
    at Ajv.compile (C:\repo\node_modules\ajv\lib\core.ts:384:22)
    at CoreSchemaRegistry._compile (C:\repo\node_modules\@angular-devkit\core\src\json\schema\registry.js:246:35)
    at CoreSchemaRegistry.compile (C:\repo\node_modules\@angular-devkit\core\src\json\schema\registry.js:231:37)
    at name (C:\repo\node_modules\@angular-devkit\architect\src\architect.js:224:43)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)

@mandarini
Copy link
Member

@marleypowell o no!!! what version of Nx are you using? do you a repro repo??

@marleypowell
Copy link

It started after this package update.
image

I can try and get a repro repo.

@mandarini
Copy link
Member

@marleypowell what command are you running that's failing?

a repro repo would be SUPER helpful, thanks!!!!

@marleypowell
Copy link

@marleypowell what command are you running that's failing?

a repro repo would be SUPER helpful, thanks!!!!

yarn nx storybook <project>

@marleypowell
Copy link

marleypowell commented Feb 2, 2024

I haven't got a repro just yet but I've narrowed it down to the ng-packagr-lite executor.

https://github.com/beeman/nx/blob/65c448727d2b6b00fea27f7f374989712cff8cec/packages/angular/src/executors/ng-packagr-lite/schema.json

image

@mandarini
Copy link
Member

mandarini commented Feb 2, 2024

Hmm just tried it, I could not reproduce: https://github.com/mandarini/storybook-nx-17.3.1 (see README)

Do you want to try to delete node_modules and install again? Maybe some weird caching issue.

@marleypowell
Copy link

Yeah I can't reproduce it in a clean repo either. Trying to work out what's different...

It's happening to everyone for the same repository and multiple people have reinstalled node_modules but it still happens 😕

@mandarini
Copy link
Member

I assume you cannot share your repo, right?

@marleypowell
Copy link

marleypowell commented Feb 2, 2024

Figured out the problem... browserTarget pointing to build vs build-storybook.

image

I remember I switched them a year ago to use just build (don't remember why). But it's a bit confusing as they both work and some of the documentation still has just build

@mandarini
Copy link
Member

Ohh thanks for pointing this out!! I suggest, specifically for Angular + Storybook, you check out the official https://storybook.js.org/tutorials/intro-to-storybook/angular/en/get-started/ documentation. The Nx Storybook docs are fine (I've written them :P) but sometimes I may have missed some update in the official Storybook docs! I should remove some of the Nx+Storybook docs that are overlapping, I guess 😬

@marleypowell
Copy link

Some of the storybook docs say to use build 😁

image

@marleypowell
Copy link

marleypowell commented Feb 2, 2024

Oh this is why I changed it from build-storybook to build.

{
  "browserTarget": {
      "type": "string",
      "description": "Build target to be served in project-name:builder:config format. Should generally target on the builder: '@angular-devkit/build-angular:browser'. Useful for Storybook to use options (styles, assets, ...).",
      "pattern": "^[^:\\s]+:[^:\\s]+(:[^\\s]+)?$",
      "default": null
    },
}

@mandarini
Copy link
Member

Ohhhh I see!!!! So you got it working now with build or build-storybook? Glad you got it working in any case hehehe

@marleypowell
Copy link

marleypowell commented Feb 2, 2024

build-storybook works (this is what NX generates). build doesn't work (this is what storybook generates/recommends). This then causes the schema issues when invoking the @nx/angular:ng-packagr-lite executor.

If your repro repo you should be able to test the difference between build-storybook and build.

@mandarini
Copy link
Member

Perfect, thanks!!

@marleypowell
Copy link

marleypowell commented Feb 2, 2024

Oh I forgot to mention, it has to be a buildable lib.

It could be useful for the docs to have a clear distinction between buildable and non buildable libs.
storybookjs/storybook#16977 (comment)

@mandarini
Copy link
Member

oh wow interesting! nice to see my comments from 2 years ago 😅 thanks for linking these

@marleypowell
Copy link

@mandarini should I create a new issue for this?

@mandarini
Copy link
Member

oh yes sure please do and I'll assign myself!

@marleypowell
Copy link

oh yes sure please do and I'll assign myself!

#21636

Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants