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

Devdocs: READMEs from calypso-ui not shown #35809

Closed
ockham opened this issue Aug 27, 2019 · 2 comments
Closed

Devdocs: READMEs from calypso-ui not shown #35809

ockham opened this issue Aug 27, 2019 · 2 comments

Comments

@ockham
Copy link
Contributor

ockham commented Aug 27, 2019

Found by @sirreal at #35768 (review).

Steps to reproduce

Go to https://wpcalypso.wordpress.com/devdocs/design/progress-bar

What I expected

The component's README to be loaded

What happened instead

It says 'No documentation available.' at the bottom of the page, even though there is https://github.com/Automattic/wp-calypso/blob/053e45ed6d06a27f258e6254025c6584550d8fc4/packages/calypso-ui/src/progress-bar/README.md

Browser / OS version

(All)

Screenshot / Video

image

Context / Source

This isn't quite as straight-forward to fix as it seems 😅

Relevant code:

const getReadmeFilePath = ( section, example ) => {
switch ( section ) {
case 'design':
return `/client/components/${ example.props.readmeFilePath }/README.md`;
default:
return `/client/${ section }/${ example.props.readmeFilePath }/README.md`;
}
};

<ProgressBar readmeFilePath="progress-bar" />

I was thinking of something like

diff --git a/client/devdocs/design/index.jsx b/client/devdocs/design/index.jsx
index e9017cce34..1ef1650067 100644
--- a/client/devdocs/design/index.jsx
+++ b/client/devdocs/design/index.jsx
@@ -252,7 +252,7 @@ class DesignAssets extends React.Component {
 					<PlansSkipButton readmeFilePath="plans/plans-skip-button" />
 					<PodcastIndicator readmeFilePath="podcast-indicator" />
 					<Popovers readmeFilePath="popover" />
-					<ProgressBar readmeFilePath="progress-bar" />
+					<ProgressBar readmeFilePath="@automattic/calypso-ui" />
 					<PromoSection readmeFilePath="promo-section" />
 					<PromoCard readmeFilePath="promo-section/promo-card" />
 					<Ranges readmeFilePath="forms/range" />
diff --git a/client/devdocs/design/search-collection.jsx b/client/devdocs/design/search-collection.jsx
index 632ce54444..e5563b05a5 100644
--- a/client/devdocs/design/search-collection.jsx
+++ b/client/devdocs/design/search-collection.jsx
@@ -37,11 +37,18 @@ const shouldShowInstance = ( example, filter, component ) => {
 };
 
 const getReadmeFilePath = ( section, example ) => {
+	const { readmeFilePath } = example.props;
+
 	switch ( section ) {
 		case 'design':
-			return `/client/components/${ example.props.readmeFilePath }/README.md`;
+			if ( readmeFilePath && readmeFilePath.startsWith( '@automattic/' ) ) {
+				// Component is in a package: treat the path as absolute.
+				console.log( readmeFilePath + 'README.md', require.resolve( readmeFilePath ) );
+				return readmeFilePath + '/README.md';
+			}
+			return `/client/components/${ readmeFilePath }/README.md`;
 		default:
-			return `/client/${ section }/${ example.props.readmeFilePath }/README.md`;
+			return `/client/${ section }/${ readmeFilePath }/README.md`;
 	}
 };
 

However, module resolution has trouble with the @automattic/calypso-ui path here (and I didn't really want to hardcode it to packages/calypso-ui).

I'm now thinking that maybe our best option is to replace the devdocs/service/content endpoint that is used to obtain those READMEs (and that's one of the oldest parts of Calypso, long predating Webpack IIRC) by a Webpack loader

// return the content of a document in the given format (assumes that the document is in
// markdown format)
app.get( '/devdocs/service/content', ( request, response ) => {
let path = request.query.path;
const format = request.query.format || 'html';
if ( ! path ) {
response
.status( 400 )
.send( 'Need to provide a file path (e.g. path=client/devdocs/README.md)' );
return;
}
if ( ! /\.md$/.test( path ) ) {
path = fspath.join( path, 'README.md' );
}
try {
path = fs.realpathSync( fspath.join( root, path ) );
} catch ( err ) {
path = null;
}
if ( ! path || path.substring( 0, root.length + 1 ) !== root + fspath.sep ) {
response.status( 404 ).send( 'File does not exist' );
return;
}
const fileContents = fs.readFileSync( path, { encoding: 'utf8' } );
response.send( 'html' === format ? marked( fileContents ) : fileContents );
} );

@sirreal
Copy link
Member

sirreal commented Aug 28, 2019

Is this something where we could plug in require.resolve and be done?

@ockham
Copy link
Contributor Author

ockham commented Dec 15, 2019

Closed by #38239.

@ockham ockham closed this as completed Dec 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants