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

Restore & add support for legacy syntax #186

Open
wants to merge 70 commits into
base: next
Choose a base branch
from

Conversation

xeho91
Copy link
Collaborator

@xeho91 xeho91 commented Jul 5, 2024

Dependencies

Requires #181 to be merged first.

Objectives

Support for legacy components for backward compatibility:

  1. <Meta>

    • codemod - <Meta> to defineMeta & insertion into <script context="module">
    • tests
  2. <Story>

    • tests
    • codemods:
      • transform let:args & let:context directives to inner children snippet block
      • transform autodocs prop to tags={["autodocs"]}
      • transform source by removing when is a shorthand or to parameters.docs.source.code
      • transform template prop to children={<template identifier>}
  3. <Template>

    • codemods - replace component with snippet block
    • tests

Add addon options - StorybookAddonSvelteCsfOptions

  • update addon's Vite plugin - add pre-transform hook
  • add option supportLegacy

Deprecation warnings, errors, messages

  • document new errors
  • Ensure deprecation warnings are visible enough while writing stories code:
    - writing the code (with TypeScript support and marking <Template> as deprecated)
  • log deprecated warnings at runtime

Update stories tests

  • Using LegacyTemplate component
  • Using LegacyStory component (and its legacy props)
  • Using Meta component

Update indexer

  • Support indexing legacy export const meta syntax
  • Support indexing legacy Meta component

TODO


📦 Published PR as canary version: 4.2.0--canary.186.b543003.0

✨ Test out this PR locally via:

npm install @storybook/[email protected]
# or 
yarn add @storybook/[email protected]

@xeho91 xeho91 added enhancement New feature or request help wanted Extra attention is needed minor Increment the minor version when merged prerelease This change is available in a prerelease. labels Jul 5, 2024
@xeho91 xeho91 self-assigned this Jul 5, 2024
@xeho91 xeho91 marked this pull request as draft July 5, 2024 05:28
@JReinhold JReinhold changed the base branch from next to experimental-support-svelte-v5 July 6, 2024 21:36
@xeho91 xeho91 changed the title Restore & support legacy <Template> component Restore & add support for legacy syntax Jul 8, 2024
@xeho91 xeho91 marked this pull request as ready for review July 15, 2024 11:56
@xeho91 xeho91 marked this pull request as ready for review August 27, 2024 07:23
@xeho91 xeho91 requested a review from JReinhold August 28, 2024 04:59
@xeho91
Copy link
Collaborator Author

xeho91 commented Aug 28, 2024

@JReinhold PR is ready for review.
Phew, there were some issues so hard to spot, because of missing type support from svelte/compiler on their AST nodes (reference)

Two requests to put attention to:

  1. If you could find any spots where we can emit warnings about using legacy syntax, it'll be awesome!
  2. Restore & add support for legacy syntax #186 (comment) This one is too hard for me to figure out at this moment. As discussed in private, is better left to user to fix very advanced cases on their own.

Copy link
Collaborator

@JReinhold JReinhold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very impressive work!

ERRORS.md Outdated Show resolved Hide resolved
ERRORS.md Show resolved Hide resolved
src/compiler/plugins.ts Outdated Show resolved Hide resolved

import { parseAndExtractSvelteNode } from '#tests/extractor';

describe(transformComponentMetaToDefineMeta.name, () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can rethink this. I'd expect that if our transformation fails for whatever reason, it will show when you load the story, and the preview will fail to render it because the syntax is bad/undefined variables are references. it could also be the svelte compiler that will fail, but I think that's more rare.

so we can attempt to catch the failing render (client-side), see if the legacy flag is enabled, and display a warning like "this is likely failing because you have logic that can't be supported with the legacy api transformer. please migrate the story file to the new api manually".

I'll think about how to do this in practice.

src/compiler/pre-transform/codemods/legacy-story.test.ts Outdated Show resolved Hide resolved
src/compiler/pre-transform/index.test.ts Outdated Show resolved Hide resolved
src/compiler/pre-transform/index.ts Outdated Show resolved Hide resolved
src/compiler/pre-transform/index.ts Outdated Show resolved Hide resolved
src/index.test.ts Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed minor Increment the minor version when merged prerelease This change is available in a prerelease.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants