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

Interactive components in playground #139

Draft
wants to merge 74 commits into
base: 1.4
Choose a base branch
from
Draft

Conversation

mariuswilms
Copy link
Member

@mariuswilms mariuswilms commented May 2, 2021

These changes introduce interactive components in playground and is based upon the design outlined in issue #75.

TODOs

@wegry

This comment has been minimized.

Copy link
Contributor

@wegry wegry left a comment

Choose a reason for hiding this comment

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

I like this approach a lot.

I am wondering how CSS handling will pan out though. Especially because what webpack calls publicPath is trickier for CSS + assets than just vanilla ESM.

@wegry
Copy link
Contributor

wegry commented May 23, 2021

How attached are we to JSX Elements being the entirety of a playground? iirc, it's supposed to be sugar to allow designers to get started with a playground without too much knowledge of JSX.

A little bit of syntactic salt would make transforming playgrounds into executable code much simpler.

The example @mariuswilms has above would be

        <Card
          headline="Headline"
          actions={
            <ButtonGroup>
              <Button>Label</Button>
              <Button primary>Label</Button>
            </ButtonGroup>
          }
        >
          Content of the card, which is usually a longer description.
        </Card>

would be a true ES module looking like

// This is a little clumsy, but it means that we don't have to do introspection of the playground code.
// It means we can use https://esbuild.github.io/api/#define to rewrite the component library path in question for serving playgrounds
import { Card } from 'component-library'

export default () => 
        <Card
          headline="Headline"
          actions={
            <ButtonGroup>
              <Button>Label</Button>
              <Button primary>Label</Button>
            </ButtonGroup>
          }
        >
          Content of the card, which is usually a longer description.
        </Card>

Because the maintainer of ESBuild isn't too keen on first party support of the automatic JSX transform, we can just inject React's default export..

https://esbuild.github.io/api/#banner allows inserting arbitrary code at the beginning of particular file extensions. However, if we want to allow more full featured state handling (hooks, for example), arbitrarily prepending export default () => will be too broad a brush.

What default exports allow for

Because the playground is (with the exception of the classic JSX runtime requiring React to be in scope) a self contained example, we bundle it with esbuild directly.

Instead of having two potential flows (top level JSX Element) or an ES Module, adding a bit of salt would allow us to just mount a component without DSK introspection. Any build time errors would be from ESBuild. Runtime errors would be through React.

Other bits

Especially for nested components in props like the above example, it might be too naive to insert props in the top level JSX element too. We might make that aspect opt in instead.

An example

import { useState } from "react"

// This lib might not be the right place for said handlers, but the idea is that it's just a handle for folks to use if they need it
import { defaultHandlers } from "@rundsk/js-sdk"
import { Button } from "component-library"

export default () => {
  const [count, setCount] = useState(0)

  return (
    <Button {...defaultHandlers} onClick={() => setCount(count + 1)}>
      {count}
    </Button>
  )
}

@mariuswilms

This comment has been minimized.

- Support the `-components` flag on `dsk`, to allow passing
  a path to a pre-build component bundle output directory.
- Provide playground handlers.
- Refactor `<Playground>`
- Use IFrame in Playground

Unrelated:

- Deprecate Node.components response property.
- Improve routing techniques, by using sub routers and allow
  us to extract route params.
- Rename flag variables.
- Pass more parent and current node into documentation components.
- Nodes now have an `id` that can be used to adress them. Although not
  from the outside.
@mariuswilms

This comment has been minimized.

@wegry

This comment has been minimized.

CONTRIBUTING.md Show resolved Hide resolved
@wegry

This comment has been minimized.

@ChristophLabacher
Copy link
Member

ChristophLabacher commented Oct 15, 2021

As discussed in our call today, these are the things we want to figure out before the PR is ready to be merged:

Rename Playground

We want to continue supporting the classic Playground DocumentationComponent from previous versions, which simply displays whatever is put inside. To do this we need to rename the new Playgrounds to something else. We discussed ReactPlayground and JSXPlayground.

Error Handling

Right now, DSK crashes when esbuild encounters an error in the JSX code. As this can easily happen when editing the code within the Playground, we do want to handle this better. A first step is to make sure DSK does not crash in this situation and to display some general error message in the Playground (instead of nothing at all). Relaying the specific error message to the frontend was deemed to be a feature that could be added later on, but is not necessary in the first version.

✅ “Unterminated string literal”

When using a template literal in a prop, esbuild displays an error. This is a minimal example:

<Playground>
  import React from 'react';

  export default () => {
    const test = "abc";
    
    return (
      <div className={`stringLiteral${test}`}>content</div>
    )
  };
</Playground>

Looking at the API response for the components in the Document, it seems there is something wrong with the extraction of the Documentation Components in our Go backend when there are template literals. The example above becomes:

"raw": "<Playground>\n  import React from 'react';\n\n  export default () => {\n    const test = \"abc\";\n    \n    return (\n      <div className={`}>content</div>\n    )\n  };\n</Playground>",

It may be related to this part in the parsing where we try to ignore inline code elements in markdown, which also use backpacks:

https://github.com/rundsk/dsk/blob/1.4/internal/ddt/node_doc_component.go#L66-L75

Path to the components

We discussed if we could make less specific requirements for the folder structure of the component library. Right now, the package.json of the components needs to be in a directory that is reached by following the path given in the import statement, taking the component path that is given as an environment variable as the root. E.g. when dsk is run with COMPONENTS=/path/to/components make dev and the components are imported with import { Component } from "@org/components" we expect the component to be at /path/to/components/@org/components.

@wegry will look into this and find out if we want to change this behavior.

Open Questions / Stretch Goals

Some open questions for later discussion:

  • Can we add support to write the JSX code not in the body of the component, but in a separate file and pass it as src? E.g. <Playground src="codeForPlayground.jsx"></Playground>
  • We want to think about caching the builds for Playgrounds
    • What do we do with the Playground.md convention of displaying a large Playground on top of the page (documentation). Can we make this possible for ReactPlaygrounds as well? Should we introduce the convention of Playground.jsx?
  • Could we watch the component directory and reload them when they change, so DSK does not have to be restarted?

@wegry
Copy link
Contributor

wegry commented Oct 16, 2021

We discussed if we could make less specific requirements for the folder structure of the component library. Right now, the package.json of the components needs to be in a directory that is reached by following the path given in the import statement, taking the component path that is given as an environment variable as the root. E.g. when dsk is run with COMPONENTS=/path/to/components make dev and the components are imported with import { Component } from "@org/components" we expect the component to be at /path/to/components/@org/components.

82741e7 has path shimming in it that should resolve this somewhat. Basically, if the COMPONENTS path base doesn't match the "name" field in the $COMPONENTS/${package.json}, we symlink it into a place that will work with NODE_PATH and esbuild. There are, of course, the normal pain points with symlinks.

@mariuswilms It uses the "write to dist" strategy that playground output used for a bit before d1bd01d. My idea there was that output paths should be stable. Maybe there's a reason for moving to temp dirs though. Ideally we wouldn't have to write to disk at all.

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.

Interactive Components in Playground UI MVP
3 participants