Docs: Extract code examples into separate files for linting

[michaelbrewer]

Description of the improvement

Summary of the proposal

Code examples should be extracted to allow for validation / linting to help prevent errors.

How, where did you look for information

See

• Papercuts priorization before we consider v2 powertools-lambda-python#1009
• Docs: Move code snippets into dedicated python files add formatting and linting, fixing any outstanding errors powertools-lambda-python#1064

Missing or unclear documentation

Improvement

• Extract examples into the docs/ folder, to allow for mkdocs to automatically pick it up
• Add autoformatter and linting against these examples
• See python pr as an example: fix(docs): extract code snippets into dedicated python files and fix examples powertools-lambda-python#1065

Related existing documentation

Related issues, RFCs

Example of documentation examples with errors in them:

• docs(all): Fix docs and examples #472

[dreamorosi]

Hi Michael, first off thanks for opening an issue to discuss the proposal.

I think this idea is valuable, I got bitten by this a few times already and having the same linting rules applied to the code snippets would help.

I have a few questions though, mainly coming from my limited knowledge of MKDocs:

1. If we separate the files can we still see them in context when authoring the docs locally?
2. Would each code snippet have its own file?
3. Would it be possible to use the same exact linting rules that we use for the rest of the project?
4. When would this linting step run?

If the answer to number 2 is yes, then if we move forward with the idea, I'd prefer to have a separate PR for each utility: i.e. one PR that addresses all the examples in docs/core/metrics, another for docs/core/logger, etc.
This way we can keep the PR size small and relatively easy to review.

I've marked this as low priority because at the moment we are focusing on the other issues marked as high before we get to GA.

With that said, after the points above have been discussed and agreed upon I'm open to review a PR from the community.

[michaelbrewer]

Hi Michael, first off thanks for opening an issue to discuss the proposal.

I think this idea is valuable, I got bitten by this a few times already and having the same linting rules applied to the code snippets would help.

I have a few questions though, mainly coming from my limited knowledge of MKDocs:

1. If we separate the files can we still see them in context when authoring the docs locally?
2. Would each code snippet have its own file?
3. Would it be possible to use the same exact linting rules that we use for the rest of the project?
4. When would this linting step run?

If the answer to number 2 is yes, then if we move forward with the idea, I'd prefer to have a separate PR for each utility: i.e. one PR that addresses all the examples in docs/core/metrics, another for docs/core/logger, etc. This way we can keep the PR size small and relatively easy to review.

I've marked this as low priority because at the moment we are focusing on the other issues marked as high before we get to GA.

With that said, after the points above have been discussed and agreed upon I'm open to review a PR from the community.

It would be option 2, each example would have it’s own file. You might have shared examples, but a PR per page makes sense.

[michaelbrewer]

@dreamorosi example PR is up #730

[dreamorosi]

Thanks for the PR, but I still would like to clear all the other points before start to review:

With that said, after the points above have been discussed and agreed upon I'm open to review a PR from the community.

Specifically I'd like to understand how/when we are going to actually lint these files.

At the moment I don't have yet a clear picture aside from just splitting the files and if there's no linting then having multiple files just makes it harder to maintain.

[michaelbrewer]

Linting would be hard if you are doing something too strict (like unused variables), but at least it should handle syntax errors, missing parameters etc..

[michaelbrewer]

@dreamorosi so far, i have already found a couple issues like the indents and the same yaml not being valid.

One option could be to configure a more lenient tsconfig to focus on compile errors. However you might still run into errors like:

[dreamorosi]

Thanks for looking into it. I'd like to hear the opinion of the other maintainers as well since it seems that there's no clear-cut way and I'm not clear on whether the overhead of having many more files is worth the gain.

Finally as I have mentioned in my first comment, this is a low priority ticket so please expect some delays in the responses.

[michaelbrewer]

Sure, i get the delay, maybe discuss with the maintainers:
What is the objectives of the code examples? Should they be runnable or only valid syntax?
It is easier to open and edit an example in VSCode with a .ts extension or just inline edit within the .md files?

The part that is frustrating to me is when an example does not have a valid syntax, or in the case of YAML not linted and has invalid indents.

[saragerion]

Thanks for opening this issue @michaelbrewer. This is an improvement that can potentially make sure our docs have better linted examples (which is a very good thing!).

The team is currently busy with other units of work with higher priority for the short term (GA), and we'll discuss about this and get back to you once we have the padding.

[michaelbrewer]

Sure thing already run a couple more issues in the docs.

So i can either file issues for them or keep it fixed in the 1 PR #730

Bugs so far for Tracer docs:

• fix indents to match the projects code style of 2 spaces (minor)
• fix the sam template.yml example, by adding missing fields (handler, AWSTemplateFormatVersion etc.)
• fix missing import for Context tracing https requests example

[dreamorosi]

Hi @michaelbrewer, I see value in the idea behind the proposal (extract the code snippet so we can apply some validation to them) and I agree with Sara, but at the moment we are extracting the files but I'm not seeing how do we actually lint/validate them.

In a previous comment you mentioned that there could be an option to set up some lenient tsconfig:

One option could be to configure a more lenient tsconfig to focus on compile errors.

If you're still interested in this, do you think you could make a proposal on how we should apply such linting? With that included in the PR we can see the results and close the circle (and so review/merge it).

[michaelbrewer]

@dreamorosi I do have something for this locally, it will mean some small changes in the examples and for the aws-sdk to also be installed. I will see if i can share it.

[dreamorosi]

Sure, the aws-sdk is already a dev dependency so it won't be an issue. Thanks for your time!

[dreamorosi]

The issue is still current and a similar effort has been made in the Python's version of this library which might serve as inspiration.

Before taking this on there are some points to discuss:

• Some of the examples show only pieces of code so they wouldn't necessarily compile, how to handle these?
• How should these code snippet be linted?
• At which point of the process should linting happen? (i.e. before merging a PR)

If anyone is interested in picking this up, please leave a comment here, then discuss your findings and proposed changes before opening a PR.

[niko-achilles]

So to recap:
We only want to focus on the code snippets written in TypeScript and inside the documentation
We can extract each snippet in its own file
We are going to tackle one documentation page in each PR (i.e. the Tracer page should be 1 PR, the Logger should be another, etc.)

And

So, essentially, we'll have ... PRs:
docs(tracer): extract code snippets in separate files
docs(logger): extract code snippets in separate files
docs(metrics): extract code snippets in separate files

@dreamorosi means

• we buy the approach to create typescript code snippets in their own files so that we can lint them.
• we drop the idea to lint typescript code snippets as markdown code blocks with the tool: eslint plugin markdown.

I agree to open new issues for

• docs(tracer): extract code snippets in separate files
• docs(logger): extract code snippets in separate files
• docs(metrics): extract code snippets in separate files

however to the issues the locations of the code snippets as separate files should be made explicit.

Now for the location of the code snippets and where the eslint configuration for code snippets live i have some questions to make see below

So, essentially, we'll have ... PRs:
chore(docs): apply current eslint rules to files under docs/snippets

@dreamorosi please note that the docs/snippets does not belong to the workspaces managed by monorepo . It would be an anti-pattern to use node_nodules and the eslint-config of the workspaces as file located here.

Questions :

1. Is it ok to install node_modules in docs/snippets ? Note the node_modules installed for the monorepo are for the workspaces and NOT for other folders like docs, etc even if i as dev. can reference them .

2. And also is it ok to create an eslint-config with the exact same rules as applied in the file, however managing the scope of this separate eslint-config file in docs/snippets by introducing the root : true field ?

@dreamorosi if Questions 1,2 is an ok then i agree with creating an issue as chore(docs): apply current eslint rules to files under docs/snippets

and also i agree with creating issues, concatenating the location of the files in the title (something like the following):

• docs(tracer): extract code snippets in separate files, docs/snippets
• docs(logger): extract code snippets in separate files, docs/snippets
• docs(metrics): extract code snippets in separate files, docs/snippets

@dreamorosi if Questions 1,2 are NOT ok then

• we buy the option to use node_modules of the workspaces also for the docs folder ( without installing node_modules in the docs/snippets folder).
• we buy the option to use same eslint-config as file located here also for the docs/snippets .

Please note Questions 1,2 are important because we need an other issue for improving the eslint configuration attributes which are NOT rules.

See here background , lessons learned from the author of eslint himself: link to blog article

The attributes like root: true, location of eslint files , package.json commands ( e.g: --resolve-plugins-relative-to ?! ) and eslintignore glob patterns are attributes that can improve the management of eslint configuration for workspaces in the monorepo and for folders outside the workspaces of the monorepo, e.g docs/snippets folder.

For the other points:

Once all code snippets are extracted, their PR merged, and we have validated that the documentation works/looks the same. We'll do another PR to start applying the linting to these files. This PR will use the existing eslint rules (regardless of whether they are OK or not).

@dreamorosi for the rules i agree . I can use the same rules for code snippets that are used for the code under packages.

Once the point above is merged & integrated with the CI/CD process, we will open a new issue to discuss any change to the eslint rules that might affect or not the whole project

@dreamorosi ok i understand for the eslint rules.

[dreamorosi]

• we buy the approach to create typescript code snippets in their own files so that we can lint them.
• we drop the idea to lint typescript code snippets as markdown code blocks with the tool: eslint plugin markdown.

Correct.

please note that the docs/snippets does not belong to the workspaces managed by monorepo . It would be an anti-pattern to use node_nodules and the eslint-config of the workspaces as file located here.

Okay, I see your point. You are right.

Questions :

1. Is it ok to install node_modules in docs/snippets ? Note the node_modules installed for the monorepo are for the workspaces and NOT for other folders like docs, etc even if i as dev. can reference them .

It's okay, but please see my question below (*).

2. And also is it ok to create an eslint-config with the exact same rules as applied in the file, however managing the scope of this separate eslint-config file in docs/snippets by introducing the root : true field ?

Yes.

---

Question: should we make the new docs/snippets folder part of the npm workspace? If we don't, the documentation will always behind the released packages on npm.

For example:

• Step 1: we are at v1.0 on NPM
• Step 2: we add a new feature to one of the utilities (this change is not released / published on npm yet)

tracer.newFeature();

• Step 3: we add a new code snippet to the docs that uses the unreleased feature
• Step 4: linting will fail because the tracer.newFeature() method doesn't exist in that installed version
• Step 5: we need to make a release, publish v2 on NPM
• Step 6: we can now update the docs (that we tried at step 3) using the published version

What do you think? Am I missing something?

[niko-achilles]

Question: should we make the new docs/snippets folder part of the npm workspace?

@dreamorosi yes , given in the Steps 1,2,3,4,5,6 described above in your comment, i understand that the lifetime of a code snippets follows the lifetime of the published code of workspace .

And also i can reference in package.json the node_modules already installed for the workspace
And also use the powertools version that are released .

And also i can use the same eslint config file , located here that is used for the workspace .

Please create the issues and mention explicit where the location of the code snippets should be for the following 4 PRs identified, so that i can start work on them:

• docs(tracer): extract code snippets in separate files. e.g. packages/docs/snippets/tracer/*.ts
• docs(logger): extract code snippets in separate files, e.g. packages/docs/snippets/logger/*.ts
• docs(metrics): extract code snippets in separate files, e.g. packages/docs/snippets/metrics/*.ts
• chore(docs): apply current eslint rules to files under docs/snippets

[dreamorosi]

I have created the first batch of issues:
#1219
#1220
#1221

Whenever you are ready you can leave a comment under each one (one at the time), so that I can assign it to you. After that you can start working on it.

I'll create the fourth later on, when we are almost done with these three.

[niko-achilles]

I'll create the fourth later on, when we are almost done with these three.

@dreamorosi
we will need an other one in context of linting and when a developer as contributor uses gitpod
This extension could be removed from https://github.com/awslabs/aws-lambda-powertools-typescript/blob/3751f318a401d85473beec085b005203f5dc6c7c/.gitpod.yml#L21

[dreamorosi]

I have created two more issues:

• Maintenance: make docs/snippets part of the npm workspace #1250
• Maintenance: apply linting to docs/snippets code snippets #1252

The first one has already been taken care of and its corresponding PR merged (#1251). The second one is up for grabs once #1245 has also been merged.

Once #1252's PR is merged, we can finally close this issue as complete.

[niko-achilles]

@dreamorosi new commit for #1245. When merged i can start with #1252 .

[dreamorosi]

It appears we had a regression when working on the Logger docs: #1253

Any chance you could fix it?

[niko-achilles]

@dreamorosi because we track in this issue the relative issues to code snippets there are some typescript compiler recommendations to improve the quality of code snippets. I have identified the following:

where: docs/snippets/logger/bringYourOwnFormatterHandler.ts

import { MyCompanyLogFormatter } from './utils/formatters/MyCompanyLogFormatter';

can be solved by:

// import { MyCompanyLogFormatter } from './utils/formatters/MyCompanyLogFormatter'; 

where: docs/snippets/logger/clearStateDecorator.ts
what:

public async handler(_event: any, _context: any): Promise<void> {
    // Persistent attributes added inside the handler will NOT be cached
    // across invocations
    if (event['special_key'] === '123456'){
      ...
    }
    ...
  }

can be solved by:

public async handler(_event: any, _context: any): Promise<void> {
    // Persistent attributes added inside the handler will NOT be cached
    // across invocations
    if (_event['special_key'] === '123456'){
     ...
    }
    ...
  }

}

where: docs/snippets/tracer/accessRootTraceId.ts
what:

export const handler = async (event: unknown, context: Context): Promise<void> => {
  try {
    ...
  } catch (err) {
    ...

    // Example of returning an error response
    return {
      statusCode: 500,
      body: `Internal Error - Please contact support and quote the following id: ${rootTraceId}`,
      headers: { '_X_AMZN_TRACE_ID': rootTraceId },
    };
  }
};

can be solved by:

export const handler = async (event: unknown, context: Context): Promise<INTRODUCE-SPECIFIC-TYPE>=> => {
  try {
    ...
  } catch (err) {
    ...

    // Example of returning an error response
    return {
      statusCode: 500,
      body: `Internal Error - Please contact support and quote the following id: ${rootTraceId}`,
      headers: { '_X_AMZN_TRACE_ID': rootTraceId },
    };
  }
};

where: docs/snippets/tracer/disableCaptureResponseMethod.ts
what:

class Lambda implements LambdaInterface {
  ...
}

can be solved by:

import { LambdaInterface } from '@aws-lambda-powertools/commons';

where: docs/snippets/tracer/putMetadata.ts
what:

export const handler = async (_event: any, _context: any): Promise<void> => {
  const res; /* ... */
  tracer.putMetadata('paymentResponse', res);
};

can be solved by:

export const handler = async (_event: any, _context: any): Promise<void> => {
  let res; /* ... */
  tracer.putMetadata('paymentResponse', res);
}; 

[dreamorosi]

With #1259 merged all the code snippets present in the documentation are now linted, this should help decrease the amount of typos/errors present in the docs.

We can close this issue as complete.

[github-actions]

⚠️ COMMENT VISIBILITY WARNING ⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.