Skip to content

Conversation

@JWhite30515
Copy link

@JWhite30515 JWhite30515 commented Sep 19, 2025

Overview

Adds support for fetching components via NS CLI

Context

DIT-11281: CLI support for components

Screenshots

Screen.Recording.2025-09-19.at.12.53.20.PM.mov

Test Plan

Now the CLI config file supports a components object, that specifies a folders field to filter down by folder.

The top-level field called components is mainly for ease of distinguishing b/w project and component sources, we could have just had a top-level field called folders but that wouldn't be immediately clear to user it's for Library (confusion b/w project folders + component folders)

This is for JSON formatting, so test with vue-i18n and i18next formats:

  • in v5-beta CLI branch, run yarn
  • Add either components: {} or components: folders: [] to existing config.yml pulling down from projects, running yarn start pull should pull down from ALL components as well as projects (with no variants filter, should see only components__base.json written to)
  • Add a variants filter, either variants: [{ id: all }] or whichever ids you want to specify, pull down, should write a components file to each variant for each group of components that has that variant (e.g. if you have components with French variant, should write to components__{french_dev_id}.json
  • all components, regardless of their folderId, get saved to same variant file (either components__base or components__variantId)
  • In the index.js file that's generated, component files are always exported last after the project-named files
  • Test removing the projects field entirely, should be allowed, on next pull it should only pull from components and write to components files
  • Test removing the components field entirely, should be allowed, on next pull it should only pull from projects and write to projects files
  • Add a folder devID to component: folders: [<{id: string}>], should only pull components from that folder and its children
  • Add rich text to a component, add richText: html to one of your output configs, should show rich text for components with rich text
  • Test specifying an extra output in outputs, that writes to another file, specify a specific set of component folders and only those components should show up in this extra output file

@JWhite30515 JWhite30515 marked this pull request as ready for review September 19, 2025 16:55
Comment on lines +41 to +43
for (let i = 0; i < data.components.length; i++) {
const component = data.components[i];
this.transformAPITextEntity(component, data.variablesById);
Copy link
Author

Choose a reason for hiding this comment

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

since we always write components to file after writing textItems, component files will always be sorted last in the generated index.js file

Copy link
Contributor

@marla-hoggard marla-hoggard left a comment

Choose a reason for hiding this comment

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

Testing went well! I left one suggestion for a small refactor, but otherwise everything looks good. Do we want to merge this as is and then open another PR with the nested folders filter or just wait for that PR to merge and get it updated here?

We will need to be careful about the timing of merging and publishing. If we can merge to v5-beta without releasing the update externally, then we can go ahead and merge any time. But I think we should wait to publish again until:

  1. This is updated with the new nested folder param + root support
  2. We publicly release component dev ids

this.variablesOutputFile = new JSONOutputFile({
filename: "variables",
path: this.outDir,
});
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious why we didn't need these before and do now?

Copy link
Author

Choose a reason for hiding this comment

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

I pulled these out into instance variables so that they can be referenced transformAPITextEntity, which handles writing to these files for a given textItem/component returned from API response

transformAPITextEntity is called on each text entity returned from API, to avoid code duplication for comps and textItems

@JWhite30515
Copy link
Author

Pushed up nit fix to split filter functions, and added some improved error logging-- I was seeing "Invalid project filters" error, which was pretty vague and made me think I'd messed up my config, but what happened was I had renamed a project's devId.

Added some extra metadata to error logs so it's clearer when we get a 400 with response data:

Screenshot 2025-09-24 at 11 10 06 AM

@marla-hoggard
Copy link
Contributor

@JWhite30515 This looks good except for the excludeNestedFolders param isn't recognized. I'm testing with that branch pulled down locally. Let's get that functionality added here so we only have to publish one new version.

We will also need to make sure we're doing versioning correctly - think we have to update the version in package.json before we merge.

@JWhite30515
Copy link
Author

JWhite30515 commented Sep 25, 2025

Now that root and excludeNestedFolders are supported, updated this PR to support them as well and upgraded version, should be good to go

Screen.Recording.2025-09-25.at.1.52.00.PM.mov

Copy link
Contributor

@marla-hoggard marla-hoggard left a comment

Choose a reason for hiding this comment

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

Still looks good! I tested again with root and excludeNestedFolders and all still looks good.

I'm approving but don't merge this just yet. We'll coordinate merging this in the proper sequence with the other related pieces on Monday.

package.json Outdated
{
"name": "@dittowords/cli",
"version": "5.0.0-beta.6",
"version": "5.0.0-beta.7",
Copy link
Contributor

Choose a reason for hiding this comment

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

We are actually moving to v8. Looks like the steps to properly release v7 last month were not followed, missing the version bump and publishing release notes 😬

Suggested change
"version": "5.0.0-beta.7",
"version": "5.0.0-beta.8",

@marla-hoggard marla-hoggard merged commit 978b517 into v5-beta Sep 29, 2025
1 check passed
@marla-hoggard marla-hoggard deleted the jerome/dit-11281-cli-support-for-components branch October 6, 2025 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants