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

Server: Initial support for @storybook/server #9722

Merged
merged 28 commits into from
Feb 4, 2020

Conversation

tmeasday
Copy link
Member

@tmeasday tmeasday commented Feb 3, 2020

I can't commit to #9250 so this is a follow up PR to try and get it over the line

@jonspalmer there are still two unresolved comments on that PR that we need to sort out:

  1. Is this a mistake or did I misunderstand: Initial support for @storybook/server #9250 (comment)

  2. Do we need to deal with this TODO: https://github.com/storybookjs/storybook/pull/9250/files/be4d06c0cd69b2ed4a79ad390852770bfd6ccfeb#diff-83aae226cfb73b52d12dfd37ae15a411?

Also a change to mdx.tsx got dropped because generateHrefWithHash doesn't exist any more. I'm not sure if that's going to be a problem but I'll have a proper play with this once the above two questions are resolved.

@tmeasday tmeasday changed the title Jonspalmer storybook server Initial support for @storybook/server (2) Feb 3, 2020
@tmeasday
Copy link
Member Author

tmeasday commented Feb 3, 2020

Oop, actually I can push to your branch, closing this.

@tmeasday tmeasday closed this Feb 3, 2020
@tmeasday tmeasday deleted the jonspalmer-storybook_server branch February 3, 2020 12:23
@jonspalmer
Copy link
Contributor

  1. I think is a bug. However, I was just pattern matching from the same code in html and web-components. Are they all bugs?
  2. We do eventually want the group support but lets add it later unless anyone things its critical

the mdx.tsx stuff really has nothing to do with this PR. That was @shilman piggy backing on the PR. I'd leave it to him to comment.

@shilman
Copy link
Member

shilman commented Feb 4, 2020

The mdx.tsx change was a temporary workaround to deal with some CORS errors we were seeing on the docs in-page linking logic. I ended up fixing it properly as part of 5.3, so those changes should be backed out if they're not already.

@tmeasday
Copy link
Member Author

tmeasday commented Feb 4, 2020

Don't re-mount the element if it didn't change and neither did the story

I guess the comment implies as "neither did the story" (which is true, the story didn't change if force render is true) that the behaviour is intended. Let's leave it for now.

@tmeasday tmeasday restored the jonspalmer-storybook_server branch February 4, 2020 05:04
@tmeasday
Copy link
Member Author

tmeasday commented Feb 4, 2020

Going to merge from this branch

@tmeasday tmeasday reopened this Feb 4, 2020
@github-actions
Copy link
Contributor

github-actions bot commented Feb 4, 2020

Fails
🚫

PR is not labeled with one of: ["cleanup","doc-dependencies:update","BREAKING CHANGE","feature request","bug","documentation","maintenance","dependencies:update","dependencies","other"]

Generated by 🚫 dangerJS against 3007fe5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants