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

Move the typedoc and typedoc-default-themes packages to a monorepo structure. #741

Closed
wants to merge 124 commits into from

Conversation

lddubeau
Copy link
Contributor

@lddubeau lddubeau commented Apr 5, 2018

Though Lerna imposes some constraints on the structure of a project, it still leaves many decisions up to the developers. One aspect is how to organize the build process for a monorepo. Solutions fall on a continuum between these two extremes:

  • have each package be built in isolation of the other packages (each package has its own build setup, e.g. its own gruntfile or equivalent),

  • have a single build system that builds everything.

There are advantages and disadvantages to each. However, in issue after issue filed on Lerna's GitHub repository, I've seen solutions that privilege the 2nd extreme. For instance, multiple issues disappear if all devDependencies are moved from the individual package.json of each package to the package.json for the whole monorepo, and using --hoist.

Greenkeeper won't be fully usable in the short term. It will process the top-level package.json. However, it is currently unable to process package.json files in subdirectories. There's a WIP that aims to provide Greenkeeper support for package.json files in subdirectories, but it is anyone's guess when it will land in a stable release.

(The loss of Greenkeeper functionality was enough for me to second-guess the choice of Lerna. I've looked for alternatives but the alternatives I found were worse than Lerna, overall. Some of them were alpha-level software. Some required major custom scripting to work, etc. Whatever Lerna's faults may be, I found it to be the least objectionable option.)

This monorepo setup was tested by publishing to a local repository and installing the resulting packages. I used them in a project of mine. They worked fine.

Misc additional notes:

  1. I noticed travis was running the test suite twice. Once when npm install executes (because that triggers the publish script, which in turn runs grunt build_and_test). A second time when travis executes the default value for the script setting, which is npm test. The new travis setup runs the test suite only once.

  2. I've added Node 8 to the Travis setup.

thegecko and others added 22 commits April 14, 2017 11:17
* Adds guard against empty `model.readme`
* Use correct name for `flags` array

This patch ensures the flags are made visible, flags such as `static` and `optional` were not appearing.

* Style changes
Icons and styles to render generic type aliases, including icon and type parameter output.
Added support for generic type aliases
The move to monorepo entails that we're using the latest themes for
testing. The latest themes introduce changes that had not yet been
committed to the typedoc repo. This commit incorporates these changes.
@aciccarello
Copy link
Collaborator

👏 Thanks for putting this together! I look forward to reviewing it soon.

Any idea on how to handle any open PRs that should be merged? Would it be best to merge those first or recreate them after?

@lddubeau
Copy link
Contributor Author

lddubeau commented Apr 5, 2018

On the basis of what I know, and a few tests I performed just now, I'd expect all ordering options to require essentially the same amount of work overall to harmonize the PRs. In the merging tests I did git was able to intelligently follow renames. Ultimately, the conflicts I saw were not primarily caused by the change in structure but by issues that would have arisen anyway, even without the structure change.

There's a difference though in terms of managing the PR submitters. If a PR made with the old structure needs some work on the part of the submitter before it can be merged, it is somewhat easier for the submitter to make any necessary changes if the old structure is still the one in use. If they need to adjust their PR after the move to the monorepo, the adjustments will probably require them to learn new things. It may be very trivial things (e.g. a file moved to a new location), or it may be more complicated. For instance, if they changed the build process in their PR, the harmonization work required will be more substantial.

But if the move to a monorepo is desirable (and I think it is), there's going to be a point at which we'll have to bite the bullet and make the move even if it causes some pain elsewhere.

@aciccarello
Copy link
Collaborator

aciccarello commented Apr 6, 2018

FYI, this would make installing from git no longer work. The package managers do have open issues to support this but none currently do (see npm/npm#2974 and yarnpkg/yarn#4725). Since it is no longer valid I think UPDATING.md should be removed.

Because of this, it might be time to finally move TypeScript to a peer dependency. That should probably be a separate PR though.

@lddubeau
Copy link
Contributor Author

lddubeau commented Apr 6, 2018

I've just pushed a new commit that fixes an erroneous target name in the scripts in package.json.

True. Installing through a reference to a remote git repository is no longer trivial once the switch to the monorepo is performed. I already use many packages that don't support installation through references to repositories (including many of my own projects), and there are issues that sometimes make installing from repo a major pain (e.g. npm/npm#4191). So I tend to not think about that scenario anymore.

I've looked at the workarounds provided with npm/npm#2974 but I did not see anything that would work correctly in our setup.

I tried a little experiment though. I modified the top package.json to make it installable (added version) and added a postinstall script:

    "postinstall": "npm run bootstrap && npm run build"

With this in place, it would be possible for someone wanting to install typedoc from a branch to install typedoc-monorepo by using a reference to its github repository and then have the package that wants to use typedoc (let's call it "the consumer") issue npm install --no-save node_modules/typedoc-monorepo/packages/<package name> for each package that the consumer needs to use. Issuing this command as needed would have to be integrated into the consumer's build process. Unfortunately, I don't see any way to do it without having to have the consumer issue the extra npm install command. So I don't see a way to do it transparently. I tried to see if I could do it from the postinstall script in typedoc-monorepo but that did not work. (There does not seem to be a reliable way to move from the directory of the package being installed to the consumer's directory.)

I tried it with a project of mine and was able to run the monorepo's version of typedoc by installing typedoc-monorepo and performing a manual npm install. However, without using this method for real it is hard to know whether there are gotchas.

@aciccarello aciccarello added this to the 1.0.0 milestone Oct 26, 2018
@lddubeau lddubeau closed this Jan 12, 2020
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.