Skip to content

feat: add default api section in manifest#40

Merged
skokaina merged 6 commits intomainfrom
feature/default-api-section-in-manifest
Apr 16, 2026
Merged

feat: add default api section in manifest#40
skokaina merged 6 commits intomainfrom
feature/default-api-section-in-manifest

Conversation

@MohamedBenTaher
Copy link
Copy Markdown
Collaborator

@MohamedBenTaher MohamedBenTaher commented Apr 10, 2026

This pull request updates the manifest generation logic and its tests to ensure that when the includeApi option is enabled but no apiType is specified, a default API section with sensible defaults is included in the generated manifest. It also adds comprehensive tests to verify this behavior.

Manifest generation improvements:

  • Added a new generateDefaultApiSection function in init-helpers.ts to generate a default API section with type: rest, port: 8000, pathPrefix: /v1, and corsOrigins: ["*"] when includeApi is true and apiType is not set.
  • Updated generateManifest to use the new default API section logic, falling back to the custom API section only when apiType is provided.

Testing enhancements:

  • Expanded and clarified tests in init.test.ts to check for the presence and correctness of the default API section when includeApi is true and apiType is not set, and to confirm its absence when includeApi is false. [1] [2]This pull request updates the manifest generation logic and associated tests to ensure that an api section is always present in the generated manifest, even when the includeApi option is set to false. The default api section uses standard values for its fields. Several tests are updated and new tests are added to verify this behavior.

Manifest generation logic changes:

  • Added a new generateDefaultApiSection function in init-helpers.ts to produce a default api section with type: rest, port: 8000, pathPrefix: /v1, and corsOrigins: ["*"] when includeApi is false.
  • Updated generateManifest to always include an api section: if includeApi is true, it uses the customized section; otherwise, it uses the default section.

Test updates and additions:

  • Updated existing tests in init.test.ts to expect the default api section when includeApi is false, including checks for the default field values. [1] [2]
  • Added a new test suite to verify that the default api section is always present and contains the expected values, even with all feature flags set to false.

@skokaina
Copy link
Copy Markdown
Contributor

Thank you @MohamedBenTaher , what is the reason we would add it by default even if the value is set to false ?

@MohamedBenTaher
Copy link
Copy Markdown
Collaborator Author

yes , you are right @skokaina , if the user sets it to false then it shouldn't be present , it should be there only if the user sets it to true and no custom value for the api is provided
i have updated the implementation as well as tests to follow

@MohamedBenTaher
Copy link
Copy Markdown
Collaborator Author

Small inconsistency worth flagging:
the default REST section (generateDefaultApiSection) uses port 8000, while the custom REST path in generateApiSection defaults to port 3000 when no apiPort is provided.
A user who opts into API without customizing gets 8000; a user who goes through the wizard and picks REST without changing the port gets 3000. These two paths should agree on the same default port .happy to align them in a follow-up if you have a preference.

@skokaina
Copy link
Copy Markdown
Contributor

Small inconsistency worth flagging: the default REST section (generateDefaultApiSection) uses port 8000, while the custom REST path in generateApiSection defaults to port 3000 when no apiPort is provided. A user who opts into API without customizing gets 8000; a user who goes through the wizard and picks REST without changing the port gets 3000. These two paths should agree on the same default port .happy to align them in a follow-up if you have a preference.

Worth reporting it as a github issue indeed, well spotted

@skokaina
Copy link
Copy Markdown
Contributor

thanks @MohamedBenTaher for your first contribution, i will merge, however please open an issue for the small inconsistency in ports it will help tracking and fixing it

@skokaina skokaina merged commit bf5afde into main Apr 16, 2026
11 checks passed
@MohamedBenTaher
Copy link
Copy Markdown
Collaborator Author

Thanks a lot @skokaina , I will definitely open an issue for it now !

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.

2 participants