[Docs] Combine All @elastic/eui Imports in the EUI Docs Demo JS Tab

[breehall]

Summary

Fixes #5104 . Previously, when multiple @elastic/eui imports were included in a module, the function to prepare and render the code for the Demo JS tab within the EUI docs would not combine some, but not all of the imports. It seems that there was logic missing to capture and combine imports that spanned multiple lines. I've organized the renderJsSourceCode method in _utils.js to combine all @elastic/eui imports and improve the readability of the function over all.

Below are images of the imports on the Forms and Form Rows page before and after the logic changes.

Before

After

Checklist

[ ] Check against all themes for compatibility in both light and dark modes
[ ] Checked in mobile
[ ] Checked in Chrome, Safari, Edge, and Firefox

• Props have proper autodocs and playground toggles
• Added documentation

[ ] Checked Code Sandbox works for any docs examples
[ ] Added or updated jest and cypress tests

• Checked for breaking changes and labeled appropriately
• Checked for accessibility including keyboard-only and screenreader modes
  [ ] A changelog entry exists and is marked appropriately

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_5533/

[thompsongl]

Cool! Looks like the consolidation works, but the formatting might cause lint errors in codesandbox.
It's a line length setting (I think) and I suspect that codesandbox will show red lines when each import is not on its own line.
This could be tricky to test because codesandbox will be outdated until these changes have been released 🙃

Perhaps I'm overthinking this so I'll let others chime in, also.

[breehall]

Cool! Looks like the consolidation works, but the formatting might cause lint errors in codesandbox.

Ah ok! I can update the consolidation to add a new line after each import (except the last) if that would give us better odds at avoiding issues with CodeSandbox.

[cee-chen]

++ to newlines between each import - even in the Demo JS tab and not just CodeSandbox, it looks visually more pleasant/easier to read

[cee-chen]

Appreciate these extra documentation notes, it makes it easy for someone new like me coming in to understand the intention & goals!

[cee-chen]

Whoops, looks like there's an issue with imports from 3rd party libs - they're not getting a closing } bracket or new lines, and react-router-dom is turning into react, router, dom

https://eui.elastic.co/pr_5533/#/tabular-content/data-grid

[breehall]

I'm not sure why or how, but it looks like this PR is adding an extra newline/whitespace at the very bottom of the demo JS code that doesn't exist on prod:

It looks like this was due to an extra space and new line in the template string responsible for displaying the rendered code. I've updated it and checked it locally and will do the same once the PR preview is complete.

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_5533/

[breehall]

I just did a spot check with the Tables and Data Grid examples and I think it looks good now.

[cee-chen]

Wohoo, can confirm the ending extra newline was fixed!

I hate to do this, but it looks like there are some double newlines occuring on a few demos:

Examples: https://eui.elastic.co/pr_5533/#/display/title, https://eui.elastic.co/pr_5533/#/display/stat

It looks like the newline is coming from source code that looks like this:

import React from 'react';

import {
  EuiTitle,
  EuiHorizontalRule,
  EuiSpacer,
  EuiCode,
} from '../../../../src/components';

So basically the preceding newline between EUI imports and non-EUI aren't getting trimmed and are causing an extra newline to exist between the imports and the actual component. It's not the worst thing in the world and IMO it's not a blocker - I'll leave that up to y'all to decide if you'd rather just move ahead or attempt to fix it now in the regex.

[breehall]

So basically the preceding newline between EUI imports and non-EUI aren't getting trimmed and are causing an extra newline to exist between the imports and the actual component. It's not the worst thing in the world so it's not a blocker - I'll leave that up to y'all to decide if you'd rather just move ahead or attempt to fix it now in the regex.

I'm going to be tackling this in an additional PR that will add another regex or function to format the non-Eui imports.

[cee-chen]

Sounds great, thanks for your patience y'all!

[thompsongl]

💟

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_5533/

[breehall]

Hey @constancecchen @thompsongl @chandlerprall - do you all mind re-reviewing this PR? I ended up making changes ti the existing functionality as well as creating new functionality for this PR.

Here's a quick run down:

1. I updated the existing Eui import formatting to take import statements that bring in only one component/function/utility into a file. In the original version of this PR, since imports were pushed onto a new line.
   

2. I added functionality to give the non-Eui imports the luxury formatting treatment! This means all imports should be formatted uniformly. This was a little tricky because there were four formats the the import statements could appear in. Below are examples of the patterns I found. From what I can tell, each pattern has been accounted for in formatting,
   A. import React from 'react';
   B. import { fake } from 'faker';
   C. import React, { Fragment, useState } from 'react';
   D. import { test1, test2 } from 'test-library';
   

3. I normalized the spacing between the import statements and the start of the demo code. In the first version of this PR, there was some additional white space added between the imports and the start of the code blocks. There's now a single new line separating the imports and the code.

Thank you all!

[cee-chen]

Rather than automatically creating newlines for each import based on the # of import, would be it be possible to check for character/line length?

It looks like both CodeSandbox and our local demo JS's switch to newlines for each import once the character length of the line goes above 81 characters (including the semicolon). Would it be possible to create a DRY helper that checks both EUI and non-EUI imports for char length > 81 instead?

No worries if not or if that's too complicated!

[breehall]

Sure, I can work on this. Do you think this will take away from the uniformity of the imports? The reason I added the logic to combine and format the non-EUI imports is because we had some code examples where imports were separated with a new line, and examples where the lines were short enough so they would just be on the same line.

If this is something that we want to adjust, would it make more sense to just remove the non-EUI import logic and allow Prettier to handle the formatting when the example is being developed?

For example, let's say we have the two import statements below:

import React, { useState } from 'react';

import { EuiFlyout, EuiFlyoutBody, EuiFlyoutHeader, EuiButton, EuiText, EuiTitle, EuiCodeBlock, } from '@elastic/eui';

If a code demo was created with these two import statements, when yarn lint-fix is run before the dev commits their code, the first import would remain the same and the second would be broken up with new lines. Please correct me if I'm wrong, but it feel like checking the length of the import statement before running the logic to separate imports with new lines is duplicating work that the linter would do for us.

This would have to be done to the EUI imports because they don't get linted, but should we be doing it for the non-EUI imports?

[cee-chen]

Do you think this will take away from the uniformity of the imports?

No, I don't think this is the same thing as the fact we have demo JS with multiple lines/imports for EUI imports. This is a Prettier/char length rule thing just IMO.

Please correct me if I'm wrong, but it feel like checking the length of the import statement before running the logic to separate imports with new lines is duplicating work that the linter would do for us.

Normally yes, but I believe this needs to be manually checked for EUI imports because unfortunately we have a bunch of demo JS doing shenanigans like this:

import { EuiFlyout } from '../../../src/components/flyout';
import { EuiCode } from '../../../src/components/code';
import { EuiText, EuiButton } from '../../../src/components';

Those lines should all get converted to @elastic/eui and combined+line checked, there isn't a realistic way a linter could handle this because it doesn't know that's what we want to do.

If this is something that we want to adjust, would it make more sense to just remove the non-EUI import logic and allow Prettier to handle the formatting when the example is being developed?

Sure, if we want! I'm not actually 100% sure what the non-EUI import logic is doing 🙈 It's nice to have a regex rule for non-EUI imports to make newlines more consistent between each import statement but I agree line char-length can be left up to Prettier.

[breehall]

Those lines should all get converted to @elastic/eui and combined+line checked, there isn't a realistic way a linter could handle this because it doesn't know that's what we want to do.

I added a helper that is being used to check the length of the import statements. When the statement is under 81 characters, the EUI imports are composed on one line. Otherwise, the imports use the existing logic to break into new lines.

Sure, if we want! I'm not actually 100% sure what the non-EUI import logic is doing 🙈 It's nice to have a regex rule for non-EUI imports to make newlines more consistent between each import statement but I agree line char-length can be left up to Prettier.

I also removed the non-EUI formatting logic, so we're just running that helper on the EUI imports and we will leave the non-EUI formatting up to Prettier.

[cee-chen]

super minor/optional note, I believe EUI should be capitalized as an initialism

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_5533/

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_5533/

[cee-chen]

Awesome, this is looking fantastic!! One comment from a quick QA - it looks like newlines between imports aren't being preserved as expected, leading to double newlines between source and final JS. Example in src-docs/src/views/date_picker/date_picker.js:

Input:

import React, { useState } from 'react';

import moment from 'moment';

import { EuiDatePicker, EuiFormRow } from '../../../../src/components';

export default () => {

Output:

import { EuiDatePicker, EuiFormRow } from '@elastic/eui';
import React, { useState } from 'react';

import moment from 'moment';


export default () => {

Is there any way to either reasonably preserve newlines between imports, or at worst trim all of them instead of just some?

[breehall]

Is there any way to either reasonably preserve newlines between imports, or at worst trim all of them instead of just some?

Sure, this is something that was included in the non-EUI import formatting logic that I had before this version of the PR. It standardized the import styling, but also removed extra spaces so they could be uniformly added when composing the EUI imports, non-EUI imports, and remaining code. Should I add this piece back in?

[cee-chen]

If it's possible to add that logic back in but just to handle newlines, that would be great!

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_5533/

[breehall]

Yep! What I did this time is include the regex for the non-EUI imports, but the only thing I did was combine them with a single new line in between statements to for standardization. There is no formatting of these imports or checks for line length for non-EUI imports.

Example from src-docs/src/views/date_picker/date_picker.js.

[cee-chen]

QA looks fantastic! 🤩 Thanks so much for your patience with my back and forth on this Bree!!

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_5533/