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

add TypeScript to deployment docs #1096

Merged
merged 4 commits into from Dec 22, 2022
Merged

add TypeScript to deployment docs #1096

merged 4 commits into from Dec 22, 2022

Conversation

stb13579
Copy link
Contributor

What does it do?

replicate the PM2 code example in the deployment page for server.js start command.

Why is it needed?

Describe the issue you are solving.

Related issue(s)/PR(s)

Let us know if this is related to any issue/pull request

@stb13579 stb13579 added source: Dev Docs PRs/issues targeting the Developer Docs pr: updated content PRs updating existing documentation content target: v4 Documentation PRs/issues targeting content from docs.strapi.io (main branch). size: small If the PR includes only 1 file with <10 lines of text or a small code fix labels Aug 30, 2022
@stb13579 stb13579 requested a review from Convly August 30, 2022 09:13
@stb13579 stb13579 self-assigned this Aug 30, 2022
@vercel
Copy link

vercel bot commented Aug 30, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
documentation ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Dec 22, 2022 at 8:18AM (UTC)
documentation-docu-mvp ❌ Failed (Inspect) Dec 22, 2022 at 8:18AM (UTC)

Copy link
Contributor

@fdel-car fdel-car left a comment

Choose a reason for hiding this comment

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

It could be confusing that in the TS code block we're not actually using TypeScript, but instead, just showing the option needed to support TS projects

I'd say that we either don't need to have a TS code block and just mention that the distDir option is needed for TS projects.

Or we can use a TS code block like the following.

// path: `./server.ts`

import strapi from '@strapi/strapi'

strapi({ distDir: '<path_to_your_out_dir>' }).start()

Though with that approach, it's worth mentioning that the user would need to run node <path_to_your_out_dir>/server.js as you can't run TS file directly with node.

@stb13579 stb13579 modified the milestones: 4.3.7, 4.4.0 Sep 6, 2022
@stb13579 stb13579 modified the milestones: 4.4.0, 4.4.1 Sep 27, 2022
@pwizla pwizla modified the milestones: 4.4.2, 4.4.3 Oct 5, 2022
@stb13579 stb13579 removed this from the 4.4.4 milestone Oct 14, 2022
@pwizla pwizla added the internal PRs created by the Strapi core team label Nov 30, 2022
Copy link
Member

@Convly Convly left a comment

Choose a reason for hiding this comment

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

LGTM, I just have a suggestion. Feel free to ignore

// path: `./server.js`

const strapi = require('@strapi/strapi');
const app = strapi({ distDir: '<path_to_your_out_dir>' });
Copy link
Member

Choose a reason for hiding this comment

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

Just a suggestion to keep consistency with the JS example

Suggested change
const app = strapi({ distDir: '<path_to_your_out_dir>' });
const app = strapi({ distDir: '<path_to_your_out_dir>' }).start();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed code example


const strapi = require('@strapi/strapi');
const app = strapi({ distDir: '<path_to_your_out_dir>' });
app.start();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
app.start();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed code example

@Convly
Copy link
Member

Convly commented Dec 5, 2022

It could be confusing that in the TS code block we're not actually using TypeScript, but instead, just showing the option needed to support TS projects

I'd say that we either don't need to have a TS code block and just mention that the distDir option is needed for TS projects.

Or we can use a TS code block like the following.

// path: `./server.ts`

import strapi from '@strapi/strapi'

strapi({ distDir: '<path_to_your_out_dir>' }).start()

Though with that approach, it's worth mentioning that the user would need to run node <path_to_your_out_dir>/server.js as you can't run TS file directly with node.

I agree we might confuse users by providing a TS code block that isn't a TS example. A callout with information regarding typescript ("if strapi typescript project then provide the distDir option") might be clearer. I'll let you judge that @pwizla @StrapiShaun .

@stb13579
Copy link
Contributor Author

It could be confusing that in the TS code block we're not actually using TypeScript, but instead, just showing the option needed to support TS projects
I'd say that we either don't need to have a TS code block and just mention that the distDir option is needed for TS projects.
Or we can use a TS code block like the following.

// path: `./server.ts`

import strapi from '@strapi/strapi'

strapi({ distDir: '<path_to_your_out_dir>' }).start()

Though with that approach, it's worth mentioning that the user would need to run node <path_to_your_out_dir>/server.js as you can't run TS file directly with node.

I agree we might confuse users by providing a TS code block that isn't a TS example. A callout with information regarding typescript ("if strapi typescript project then provide the distDir option") might be clearer. I'll let you judge that @pwizla @StrapiShaun .

I like this idea. I added a callout instead and think it is much clearer. Thanks for the suggestion.

@stb13579 stb13579 merged commit 2fa5f1f into main Dec 22, 2022
@stb13579 stb13579 deleted the dev/typescript-deployment branch December 22, 2022 17:40
@strapi-bot
Copy link

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/new-documentation-release-v4-5-5/24573/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal PRs created by the Strapi core team pr: updated content PRs updating existing documentation content size: small If the PR includes only 1 file with <10 lines of text or a small code fix source: Dev Docs PRs/issues targeting the Developer Docs target: v4 Documentation PRs/issues targeting content from docs.strapi.io (main branch).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants