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

Expose unified util to pack TypeScript plugins #5564

Open
Josh-Cena opened this issue Sep 13, 2021 · 6 comments · May be fixed by #5612
Open

Expose unified util to pack TypeScript plugins #5564

Josh-Cena opened this issue Sep 13, 2021 · 6 comments · May be fixed by #5612
Labels
domain: dx Related to developer experience of working on Docusaurus sites proposal This issue is a proposal, usually non-trivial change

Comments

@Josh-Cena
Copy link
Collaborator

Josh-Cena commented Sep 13, 2021

💥 Proposal

As more of the official packages are migrated to TS, I start to notice duplicated logic in the packing process, but not following this long process of compiling -> copying static assets -> double transpilation can cause some troubles.

The first one is less of a concern, although more likely to occur: when the theme folder contains .css files, they are not copied to the dist folder by tsc, you would either need to transpile by babel with allExtensions: true, or you would need to use the copyUntypedFiles script (which we use a lot—duplicated in every package). Neither is very convenient for a TS plugin developer. However, the plugin author cannot get away without using one of the two techniques, because missing .css files would make the code not run at all.

The second one can be more problematic. The TypeScript theme components are transpiled twice, once only with the types stripped and once to actual commonjs code. Now imagine the following case:

  • The developer of a plugin writes the plugin in TS;
  • Unaware of the double-transpile workflow, the plugin is only built with tsc and distributed;
  • A user uses the plugin, and decided to swizzle a theme component, but uses the JS version;
  • The user will receive an unreadable component.

This can go away unnoticed because for normal users who don't swizzle, the plugin runs correctly. The above case is very rare as of now, but can be likely in the future with a better plugin ecosystem. Also, many official plugins have their own theme components, and most of them are currently kept as JS to bypass this issue—still not optimal for TS-perfectionists like me. If we can expose a util to transpile the theme components to human-readable JS automatically, things would be a lot easier for the plugin authors.

I propose we have a docusaurus-plugin build command, meant to be used by plugin authors, which runs Babel and generates a nice directory structure automatically, maybe like this:

plugin
├── dist
│   ├── index.js
│   └── theme
│       └── Component.js
├── js-theme
│   └── Component.js
└── src
    ├── index.ts
    └── theme
        └── Component.ts

We can use this internally to reduce the logic duplication, so it's better if it gets into this repo.

We may even provide a docusaurus-plugin init utility to scaffold a plugin package, just like docusaurus init.

Have you read the Contributing Guidelines on issues?

Yes

Edit. Once #5816 lands, we don't need to transpile twice. Hence, the major focus of this tool shifts towards an ergonomic DX: hot reloading for CSS edits, type-checking integration, etc., than actually bundling & building.

@Josh-Cena Josh-Cena added status: needs triage This issue has not been triaged by maintainers proposal This issue is a proposal, usually non-trivial change labels Sep 13, 2021
@Josh-Cena Josh-Cena changed the title Expose unified util to transpile theme components Expose unified util to pack TypeScript plugins Sep 13, 2021
@Josh-Cena
Copy link
Collaborator Author

Made a proof-of-concept package: https://www.npmjs.com/package/@joshcena/docusaurus-plugin-utils

@slorber
Copy link
Collaborator

slorber commented Sep 21, 2021

Yes I totally agree with this idea and would be happy to have such util in core and use it internally (before exposing it to plugin authors a bit later).

The copyUntypedFiles has always been a pain for me + watch mode for css files does not even work everywhere currently (like in theme-common), which is really painful for some devs

@Josh-Cena
Copy link
Collaborator Author

@slorber Turns out that this is not that easy and probably requires playing with our infrastructure. If you look at my implementation: https://github.com/jc-verse/docusaurus-plugin-utils there are several things I'm not happy with:

  • The build function calls an internal API @babel/cli/lib/babel/dir which will probably break in future versions. (See Add transformDirectoryAsync method to @babel/core babel/babel#9224 maybe)
  • To build the other plugins this package has to be built first in our monorepo, which I don't know how to ensure—it seems Lerna builds packages in no particular order, and I can't find where it's configured. I've tested my util in multiple Docusaurus projects and they seem to be working well, but I'm not sure about how it works in a monorepo.
    • Of course, I want to write this util in TypeScript, because JavaScript is whacky.
  • We run prettier after this, but Prettier has no API to format a directory either. We would still need to walk the directory ourselves.

@slorber
Copy link
Collaborator

slorber commented Sep 24, 2021

The build function calls an internal API @babel/cli/lib/babel/dir which will probably break in future versions.

If we don't expose this package publicly this shouldn't be a problem for internal usage, until we find a solution to provide to the community.

At first I'd prefer to keep this package undocumented until we are sure it works well for us

To build the other plugins this package has to be built first in our monorepo, which I don't know how to ensure—it seems Lerna builds packages in no particular order, and I can't find where it's configured. I've tested my util in multiple Docusaurus projects and they seem to be working well, but I'm not sure about how it works in a monorepo.

Lerna will build according to the dependency graph. If the new package is listed in theme deps it will be build first.

That's also why Lerna doesn't allow any dependency cycle otherwise it can't compute a build order.

Of course, I want to write this util in TypeScript, because JavaScript is whacky.

For such package using JS doesn't feel like a big deal IMHO. It's better to have it in JS than do not have it 🤪

We run prettier after this, but Prettier has no API to format a directory either. We would still need to walk the directory ourselves.

glob dir/** + forEach + format should work fine

@Josh-Cena Josh-Cena linked a pull request Sep 25, 2021 that will close this issue
5 tasks
@Josh-Cena Josh-Cena removed the status: needs triage This issue has not been triaged by maintainers label Oct 30, 2021
@armano2
Copy link
Contributor

armano2 commented Nov 23, 2021

in teory we could use overrides for now, and bundle everything with babel in one go, but currently there is no way to apply this to every package, as there is no clear spit between server and client.

I read somewhere proposal (maybe comment) about refactoring structure of all packages to:

  • src/client
  • src/server
    but i don't know if that's something what you are planning to do.

Most packages currently follow rule: if theme than this is client, but there are exceptions to that, for example gtag.ts

if we have clear folder boundaries for client/server code, i don't mind writing eslint rule that can enforce this :)
edit: there is already plugin for this: eslint-plugin-boundaries

@slorber
Copy link
Collaborator

slorber commented Nov 24, 2021

Agree we should move into that direction and have client/server/common in each plugin.
I've also used eslint restricted import in a few apps to avoid unwanted imports/dependencies, didn't know about this boundaries plugin

@Josh-Cena Josh-Cena added the domain: dx Related to developer experience of working on Docusaurus sites label Mar 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain: dx Related to developer experience of working on Docusaurus sites proposal This issue is a proposal, usually non-trivial change
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants