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

feat: new init template classic-typescript #5233

Merged
merged 43 commits into from
Aug 6, 2021
Merged

Conversation

Josh-Cena
Copy link
Collaborator

@Josh-Cena Josh-Cena commented Jul 28, 2021

Motivation

Users don't have to follow the documentation every time a new website is initialized to add Typescript support. Now the TS template is available via the --typescript flag on init.

Also some minor refactors.

A downside is duplicated code between classic and classic-template folders. We can either use symlinks or copy the duplicated files from classic instead of classic-typescript—discussions welcomed.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

The template itself builds successfully. The CLI works as intended by running

yarn docusaurus-init init --typescript

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Jul 28, 2021
@netlify
Copy link

netlify bot commented Jul 28, 2021

✔️ [V2]
Built without sensitive environment variables

🔨 Explore the source changes: 40b6d51

🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/6101440266591500085b6086

😎 Browse the preview: https://deploy-preview-5233--docusaurus-2.netlify.app

@github-actions
Copy link

github-actions bot commented Jul 28, 2021

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟢 Performance 91
🟢 Accessibility 98
🟢 Best practices 100
🟢 SEO 100
🟢 PWA 95

Lighthouse ran on https://deploy-preview-5233--docusaurus-2.netlify.app/

@netlify
Copy link

netlify bot commented Jul 28, 2021

✔️ [V2]
Built without sensitive environment variables

🔨 Explore the source changes: f89d75c

🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/610d1a4a599bc80008306915

😎 Browse the preview: https://deploy-preview-5233--docusaurus-2.netlify.app

@Josh-Cena Josh-Cena force-pushed the ts-init branch 2 times, most recently from edd4c27 to 1262b59 Compare July 29, 2021 01:22
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Copy link
Collaborator

@slorber slorber left a comment

Choose a reason for hiding this comment

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

Thanks, that looks nice

In the future, I'd like the ability to avoid duplicating docs and share the same docs/blog for all themes. The FB variant can also be quite similar to the classic template + some tweaks (not sure which 😅).

So, we'll probably move away from using symlinks in the future and have a shared/docs folder, but in the meantime that looks fine

generateExamples.js Outdated Show resolved Hide resolved
packages/docusaurus-init/src/index.ts Show resolved Hide resolved
## Setup {#setup}

Docusaurus supports writing and using TypeScript theme components. To start using TypeScript, add `@docusaurus/module-type-aliases` and some `@types` dependencies to your project:
To start using TypeScript, add `@docusaurus/module-type-aliases` and some `@types` dependencies to your project:
Copy link
Collaborator

@slorber slorber Aug 5, 2021

Choose a reason for hiding this comment

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

do we need those deps in the init project as well? Does not seem mandatory

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't really know, I suspect yes. Users are very likely to run into these types sooner or later, although I don't think the react types are used in the init template...

Initially, I was just creating template that's exactly what would be like if the user follows this doc

slorber and others added 6 commits August 5, 2021 19:56
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
generateExamples.js Outdated Show resolved Hide resolved
@slorber
Copy link
Collaborator

slorber commented Aug 6, 2021

I was wondering: wouldn't it be simpler to directly symlink the docs/blog folder instead of individual files?

I'd like to have all templates (also fb/bootstrap) sharing the same docs/blog/sidebar config instead of copies that we have to keep in sync (and yes, it's not in sync currently 😅 ), your symlinks system + dereference can be a good fit. I think moving init template content outside of the /template folder could even make sense, and the classic template would also use symlinks to src/content/docs and src/content/blog ?

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
@Josh-Cena
Copy link
Collaborator Author

I was wondering: wouldn't it be simpler to directly symlink the docs/blog folder instead of individual files?

That was just me playing it safe (because I'm a noob at this and can't guarantee it works). I would also argue that this creates a more straightforward directory structure, and makes room for template-unique pages in the future.

I'd like to have all templates (also fb/bootstrap) sharing the same docs/blog/sidebar config instead of copies that we have to keep in sync (and yes, it's not in sync currently 😅 ),

I tried a bit on this, but gave up because I suspect the FB template would require the copyright header in sidebars.js yet I don't want the header to appear on other templates...

@slorber
Copy link
Collaborator

slorber commented Aug 6, 2021

this creates a more straightforward directory structure, and makes room for template-unique pages in the future.

No I think we mostly want the same content everywhere, which is the basic 5min tutorial

I tried a bit on this, but gave up because I suspect the FB template would require the copyright header in sidebars.js yet I don't want the header to appear on other templates...

Yes I don't want that too, but maybe only the sidebar can be duplicated in the FB template for now

@Josh-Cena
Copy link
Collaborator Author

it's already in the base config

Didn't know that. It seems all the options are there actually😅 Previously I didn't have the base config properly installed. I've removed all compiler options now

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
@Josh-Cena
Copy link
Collaborator Author

Josh-Cena commented Aug 6, 2021

So should I create the shared folder in this PR as well, or should we work on this in another PR?

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
@slorber
Copy link
Collaborator

slorber commented Aug 6, 2021

So should I create the shared folder in this PR as well, or should we work on this in another PR?

I think we can merge this one first and improve later, this will be easier to review

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
@slorber
Copy link
Collaborator

slorber commented Aug 6, 2021

LGTM 👍 thanks

@slorber slorber changed the title feat: init template with Typescript feat: new init template classic-typescript Aug 6, 2021
@slorber slorber merged commit 28e2599 into facebook:master Aug 6, 2021
@Josh-Cena Josh-Cena deleted the ts-init branch August 7, 2021 00:06
@Josh-Cena
Copy link
Collaborator Author

Josh-Cena commented Aug 7, 2021

Unfortunately seems NPM doesn't support symlinks: npm/npm#6703

We do need to make the init script copy the duplicate files from classic instead...

FYI, create-react-app template just duplicates the files around, but I think we can do better

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: new feature This PR adds a new API or behavior.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants