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

Fix/pro 173 #99

Merged
merged 5 commits into from
Jun 12, 2024
Merged

Fix/pro 173 #99

merged 5 commits into from
Jun 12, 2024

Conversation

christianzoppi
Copy link
Contributor

@christianzoppi christianzoppi commented May 31, 2024

This PR fixes some problems that happen when running pull-components with the --separate-files parameter. Some files will be corrupted during the asynchronous save. The code of the CLI doesn't even wait for the writing operation to be over.

Pull request type

Jira Link: INT-

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Other (please describe):

How to test this PR

In the same parent folder as the storyblok-cli one, create one called data, then run:
Run npm run dev pull-components -- --space 88751 --separate-files --path ./../data/
You should get no errors. Now open any JSON file and inspect the content. The content should be correct.

What is the new behavior?

Files from pull-components command are stored synchronously

Other information

Copy link

Pull components save bug

@Edo-San
Copy link
Contributor

Edo-San commented Jun 3, 2024

Thanks for this PR @christianzoppi 🙏
I finally had some time to look more deeply into this.

This weird behaviour we're currently getting is probably caused by concurrent writeFile calls on the same files.
If we take a look at the Default-88751.json file that is created, for example, which is a file for a Preset, the result is a weird "merge" of two presets:

  • A preset named Default for the enterprise_group_cta blok
  • A preset named Default for the link_board component

Making those requests synchronous would mean that we'd be getting well-formatted JSONs, but this doesn't solve the underlying issue: the CLI is not handling presets for different bloks that share the same name.

To solve this issue we should probably come up with a more refined naming strategy, or a folder-based structure.

Something like:

[PREFIX - blok name]-[preset name]-[spaceID].json

examples
link_board-Default-88751.json
enterprise_group_cta-Default-88751.json

or

[FOLDER - blok name, containing both blok schema and its presets]
    [preset name]-[spaceID].json
    [blok name]-[spaceID].json

examples
/link_board
  link_board-88751.json
  Default-88751.json
/enterprise_group_cta
  enterprise_group_cta-88751.json
  Default-88751.json

If this is going to be a breaking change (albeit a fix), we might consider taking some time to think about this whole naming update more thoroughly and group it with other breaking changes, so that we don't release a bunch of new majors, one for each breaking change.

@christianzoppi
Copy link
Contributor Author

Hey @Edo-San, like we agreed in DM I added a parameter for the pull-components command to prefix the presets names with the name of the component. I updated also the readme file. I also stressed in the readme that in a future major release this will become the default behavior. Could you please test it with this parameter?

Copy link
Contributor

@Edo-San Edo-San left a comment

Choose a reason for hiding this comment

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

LGTM 👍 At this point, I don't think it's even necessary to swap writeFile with its sync counterpart but the end result is basically the same 😊 We can leave it as it is right now, knowing that this is going to be rewritten from the ground up, eventually.

README.md Outdated
@@ -96,12 +96,14 @@ $ storyblok pull-languages --space <SPACE_ID>

Download your space's components schema as json. By default this command will download 2 files: 1 for the components and 1 for the presets; But if you pass a flag `--separate-files or --sf` the command will create file for each component and presets. And also you could pass a path `--path or -p` to save your components and presets.

It's highly recommended to use also the `--prefix-presets-names` or `-ppn` parameter if you use `--separate-files` because it will prefix the names of the individual files with the name of the component. This feature solves the issue of multiple presets from different compoentns but with the same name, being written in the same file. In a future major version this will become the default behavior.
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: compoentns instead of components

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, thank you!

@christianzoppi christianzoppi merged commit 1d96b5a into master Jun 12, 2024
1 check passed
@christianzoppi christianzoppi deleted the fix/PRO-173 branch June 12, 2024 08:24
Copy link

🎉 This PR is included in version 3.32.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pull-components --separate-files flag mix/overwrites presets and components
3 participants