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

[Proposal] Change design of package @docusaurus/core. #4364

Closed
BogdanDor opened this issue Mar 8, 2021 · 3 comments
Closed

[Proposal] Change design of package @docusaurus/core. #4364

BogdanDor opened this issue Mar 8, 2021 · 3 comments
Labels
proposal This issue is a proposal, usually non-trivial change

Comments

@BogdanDor
Copy link
Contributor

Change design of package @docusaurus/core.
This code has implicit dependencies and is hard for newbs. Logic this package interwined with infrastructure code. Modules interact with file system, network and third-party libraries. Tests also have problems. By tests hard understand what does package. Tests is written for individuals files and they probably check implementation, not interface, that user interact with one.

💥 Proposal

To fix this problems, propose remove external dependencies from modules from folder commands. And later move features to custom webpack loaders and plugins.
Steps necessary to perform for that.

  1. Write integration tests. Tests should checks commands corecteness interact with external environment.

  2. Replace code that interact with fs, network and 3rd-party libraries to wrappers.

  3. Change requirements for commands. That modules shouldn't interact with infrastructure now, only call explicit injected functions. Move wrappers to individual files. Pass every dependency explicit to constructor. In tests instead of external dependencies use mocks.

  4. Source tests could be removed because module tests from commands and integration tests should cover all possible cases.

  5. Features from commands step-by-step move to webpack loaders and plugins. Requirements and сonsequently
    tests should change also step-by-step. Finally, instead of methods createConfig and modules commands should be files with webpack config.

Have you read the Contributing Guidelines on issues?

Yes

@BogdanDor BogdanDor added status: needs triage This issue has not been triaged by maintainers proposal This issue is a proposal, usually non-trivial change labels Mar 8, 2021
@slorber
Copy link
Collaborator

slorber commented Mar 8, 2021

Sorry but I have a hard time understanding this proposal.

If you have a concrete thing you can improve PRs are welcome, but please don't refactor everything at once because it's really hard to review for me.

@Josh-Cena
Copy link
Collaborator

The idea is solid although the language is hardly understandable.

By my understanding, the OP is asking for:

  • Tests to be fully E2E instead of testing internal impl; (which I partially agree, E2E tests should be improved, but unit tests are also necessary)
  • Separate side-effects (fs, web request...) from pure core logic (I don't know how that's possible but the idea is sound, like how functional programmers do things);
  • The CLI should only call Node.js / Webpack handles (also requested in [Proposal] Docusarus NodeJS api #4841).

@Josh-Cena Josh-Cena removed the status: needs triage This issue has not been triaged by maintainers label Oct 30, 2021
@Josh-Cena
Copy link
Collaborator

Actually, closing in favor of #3056 and #4841

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal This issue is a proposal, usually non-trivial change
Projects
None yet
Development

No branches or pull requests

3 participants