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

[DO-NOT-MERGE] Proof of concept TypeScript theme support with auto-generated human readable js #2614

Closed
wants to merge 1 commit into from
Closed

[DO-NOT-MERGE] Proof of concept TypeScript theme support with auto-generated human readable js #2614

wants to merge 1 commit into from

Conversation

SamChou19815
Copy link
Contributor

@SamChou19815 SamChou19815 commented Apr 16, 2020

Motivation

This diff shows how I think TypeScript support for themes can be implemented. This is not production ready. This diff only shows how we can generate nice and human-readable js. It doesn't go further to implementing any swizzling logic proposed in #2470.

It uses a simple babel configuration that strip away the typescript types. The generated file's code style is not completely preserved, but it's still very readable. This is what you can get when you run yarn strip-ts-types in @docusaurus/theme-classic:

(Generated from src/theme/NotFound.tsx)

/**
 * Copyright (c) Facebook, Inc. and its affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */
import React from 'react';
import Layout from '@theme/Layout';

function NotFound() {
  return <Layout title="Page Not Found">
      <div className="container margin-vert--xl">
        <div className="row">
          <div className="col col--6 col--offset-3">
            <h1 className="hero__title">Page Not Found</h1>
            <p>We could not find what you were looking for.</p>
            <p>
              Please contact the owner of the site that linked you to the
              original URL and let them know their link is broken.
            </p>
          </div>
        </div>
      </div>
    </Layout>;
}

export default NotFound;

(You can see from above example that the parentheses around jsx has been stripped away as well. If we really care about the style, we can run prettier on those generated js file after the babel step.)

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Run yarn workspace @docusaurus/theme-classic strip-ts-types to see what's generated

Related PRs

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)

@docusaurus-bot
Copy link
Contributor

Deploy preview for docusaurus-2 ready!

Built with commit ed3b5ac

https://deploy-preview-2614--docusaurus-2.netlify.app

@fanny fanny added the status: don't merge yet This PR is completed, but we are still waiting for other things to be ready. label Apr 16, 2020
import Layout from '@theme/Layout';

function NotFound() {
function NotFound(): ReactElement {
Copy link
Contributor

Choose a reason for hiding this comment

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

One concern I have is that in the past, I have had a lot of issues with typing react components, because it seems like there are a million different types for components, e.g. React.Component, React.Element, and React.ReactElement (never actually seen this one before). Might this potentially cause type mismatches?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just added this to show Babel type stripping works. I don't think we will keep it in final release.

Copy link
Contributor

@fanny fanny left a comment

Choose a reason for hiding this comment

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

I wouldn't worry about the return of a render function, since we already know that is a React element or a React node. Although I know that is a POC, I think that we should more concerned about functions types and prop types.

@SamChou19815 SamChou19815 deleted the ts-theme-components-proof-of-concept branch May 22, 2020 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA status: don't merge yet This PR is completed, but we are still waiting for other things to be ready.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants