Skip to content

Modernize module namespace with (include_subdirs qualified)#244

Draft
balat wants to merge 36 commits into
masterfrom
modernize
Draft

Modernize module namespace with (include_subdirs qualified)#244
balat wants to merge 36 commits into
masterfrom
modernize

Conversation

@balat

@balat balat commented Apr 9, 2026

Copy link
Copy Markdown
Member

Summary

  • Replace (wrapped (module_name Ot)) with (include_subdirs qualified) + (wrapped false)
  • Move .eliom/.eliomi files into Ot/ subdirectory
  • Use --server-objs-dir in ocsigen-dune-rules for direct .cmo paths
  • Add ocsigen-dune-rules as opam dependency
  • Remove vendored ocsigen-dune-rules

Module names (Ot.Buttons, Ot.Spinner, etc.) are unchanged.

Part of the Ocsigen namespace modernization effort.

🤖 Generated with Claude Code

@balat balat force-pushed the modernize branch 6 times, most recently from c1db1e6 to 791a4f1 Compare May 4, 2026 10:42
@balat balat force-pushed the modernize branch 7 times, most recently from b1f364b to c3cf4d8 Compare May 29, 2026 17:06
@@ -0,0 +1,235 @@
(subdir Ot

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@Julow is there a way to generate this file instead of committing it?
I tried several solutions (dynamic_include in the same dune, in a subdir, in the parent) but always get "Depenency cycle".
Same issue in Ocsigen Start

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This file shouldn't be committed and it isn't needed by ocsigen-dune-rules. This is new in your patch.
I have plans to support wrapped libraries in ocsigen-dune-rules soon.

Comment thread src/widgets/dune
(_
(flags
(:standard -w +A-4-9-16-27-32-33-39-44-48-60-70))))
(:standard -w +A-4-9-16-22-27-32-33-39-44-48-60-70))))

@balat balat May 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@Julow How can we avoid that?

Warning 22 (The type of this injected value contains a type variable that could be wrongly inferred) is triggered by many widgets.

For example Ot.Form.password_toggle must have concrete type [`Input] elt instead of polumorphic [< Html_types.input] elt we would like. Otherwise we get:

Error: The implementation src/widgets/ot_form.pp.eliom
does not match the interface src/widgets/ot_form.pp.mli:
Values do not match:
val password_toggle :
([< Html_types.input ] as '_weak1) Eliom.Content.Html.elt ->
[> Html_types.div ] Eliom.Content.Html.elt
is not included in
val password_toggle :
[< Html_types.input ] Eliom.Content.Html.elt ->
[> `Div ] Eliom.Content.Html.elt

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a known issue and the best fix is to go over all the warnings and fix them.

balat and others added 15 commits June 22, 2026 14:41
js_of_ocaml must not be a runtime dependency of the server library,
only a compile-time include for type definitions. Otherwise, loading
the library via Dynlink (ocsigenserver in lib mode) fails with
"Unimplemented Javascript primitive caml_pure_js_expr".

Use -I include flags (via type_includes) instead of libraries, like
Eliom does.
The previous commit migrated all uses of Eliom_xxx to the new
Eliom.Xxx wrapped style, which is only available in eliom 13.
Use [>= "13.0~"] (not [>= "13.0.0"]) so opam also accepts the
pre-release [13.0~dev] version shipped by the eliom modernize
branch (in opam semver, [13.0~dev < 13.0.0]).

Cap at [< 14.0.0] so CI does not try to build against future
eliom majors that may break this.
Drop the vendored copy of [ocsigen-dune-rules/] (and the
[(vendored_dirs ...)] directive) and depend on the published
[ocsigen-dune-rules] opam package instead. The published CLI now
exposes [gen] / [check-modules] sub-commands, so update the
[dune.client] generation rule from [ocsigen-dune-rules .] to
[ocsigen-dune-rules gen .].
Rename source files (ot_form.eliom -> form.eliom, etc.), update
internal references, and configure dune to expose the modules under
a hierarchical Ot.X namespace (Ot.Form, Ot.Carousel, ...) using
[(include_subdirs qualified)] + [(wrapped false)] with files in an
[Ot/] subdirectory.

Split [src/widgets/dune] into a server-only file and a new
[src/widgets/client/dune] for the client lib. The [dune.client]
generation rule moves with the client lib and passes
[--subdir Ot --server-objs-dir ../.widgets_server.objs/byte] to
[ocsigen-dune-rules gen] so [-server-cmo] resolves unambiguously to
the server lib's [.cmo].

Also fix a stale lowercase [ot_noderesize.attach] reference in
Ot.Noderesize's doc example.
Replaces the local ad-hoc ICON_HTML signature with the shared
signature now exposed by Eliom. Same goal (avoid the
[module type of Eliom.Content.Html.F] strengthening bug in wrapped
libraries), but with no local boilerplate.
Add a documented icons.eliomi declaring the [Ot.Icons.S] signature
exported by [Make] and the two ready-made instantiations
[Ot.Icons.F] and [Ot.Icons.D]. The implementation gains the matching
[module type S] declaration so that the new interface type-checks;
no behaviour change.

Each predefined icon (user, plus, spinner, shutdown, config,
signout, close, question) gets its own ocamldoc comment naming the
CSS class it relies on.
Provides old Ot_xxx module names as aliases to new Ot.Xxx modules
to ease migration of existing code.
src/widgets/dune and src/widgets/client/dune use lwt_ppx in their
preprocess (pps), but lwt_ppx is a separate opam package from lwt and
was not pulled in transitively, breaking installation in a fresh switch.
build/gen_wikidoc.sh runs eliomdoc with the wikidoc plugin against
the wrapped Ot.* modules in src/widgets/Ot/, producing .wiki files
in _build/doc/dev/api/{server,client}/. The script handles the
strengthening of wrapped module names by generating short-name
[include Ot__X] aliases in a temporary directory before invoking
eliomdoc, so that doc cross-references like [Form.X] resolve
correctly.

doc/{server,client}.indexdoc group the toolkit modules into
Widgets / DOM utilities / Lib so the website navigation reflects
that grouping instead of a flat list.

The README is updated to point to the script. The output is meant to
be committed on the wikidoc branch under doc/dev/api/.
W22 (preprocessor warning) is emitted by the Eliom PPX on some of
the wrapped widgets. The warning is not actionable from the toolkit
side and would block -warn-error. Until the upstream warning is
fixed, silence it locally.
Major version bump: the module renames (Ot_xxx -> Ot.Xxx) and the
required move to Eliom 13 (Eliom.Xxx wrapped style) are breaking
changes for existing users. The legacy Ot_xxx names remain
available via the new ocsigen-toolkit-compat package.
Convert the wikicreole manual (intro) to odoc .mld and add a package
landing page (index.mld). The (documentation (package ocsigen-toolkit))
stanza compiles them in the same odoc run as the API, so {{!page-X}} and
{!Module} references resolve, and the manual ships on ocaml.org too.

Source: wikidoc:doc/dev/manual (dev module names), converted with
wodoc convert --odoc-refs.
Replace the per-version build.sh (in ocsigen.github.io) with a single declarative
doc/wodoc consumed by 'wodoc build': (client-server ...) server/client sides from
doc/{server,client}.indexdoc (wrapper Ot), (odoc-driver ocsigen-toolkit) builds
the API with odoc_driver --remap on the installed package, (hosted ...) rewrites
sibling Ocsigen xrefs to relative links. dev-only CI (deploy dev, only from
master); stable versions built by hand. NB: needs the odoc_driver base_args fix.

Verified with wodoc build: 81 pages, per-side nav/colour/switch, 0 template holes.
@balat balat force-pushed the modernize branch 2 times, most recently from 67c0cee to 10455e3 Compare June 25, 2026 14:59
balat and others added 9 commits June 25, 2026 19:35
Landing -> intro.html (ocsigen-toolkit/index.html does not exist); links to
eliom and ocsigen-start absolute (/eliom/..., /ocsigen-start/...).
Run the Documentation workflow with a version input to freeze the dev docs
currently on gh-pages as /<version>/ and repoint `latest` at it (wodoc release,
which also refreshes versions.json). No rebuild. Push / version-less runs keep
rebuilding dev only.
Match the new wodoc nav format (manual nav declared in doc/wodoc instead of an
overlaid wikicréole menu.wiki); remove the CI step that fetched it from wikidoc.
Keep the (hosted ...) table in sync with master: add the js_of_ocaml/tyxml
(subdir layout) and reactiveData (root) entries so cross-project links point
to ocsigen.org. Avoids reverting them when this doc branch merges.

Requires wodoc with the `subdir` layout (ocsigen/wodoc#4).
index.mld already is the package home page (it links to the introduction and
to the API). Add @children_order so the introduction comes before the API
modules in the ocaml.org sidebar, and point the ocsigen.org landing at the
home page instead of the introduction.
Follow-up to #255: index.mld (a short home page) and intro.mld (installation,
programming style, CSS, widgets overview) overlapped. Fold the introduction's
content into index.mld so there is a single home page, and drop intro.mld and
its navigation entry.
Bring the modernize branch in line with master: convert the root-relative
ocsigen.org links to odoc cross-package references (quoting hyphenated page
names), matching master so a future merge does not reintroduce the old URLs.
Like master, the Ocsigen Start link targets page-index (its intro page is merged
into the home here).
The merge of intro into index (#256) dropped the paragraph linking to Ocsigen
Start's demo application that master's introduction has. Restore it.
The API docs are now built with odoc/wodoc (see doc/wodoc and the
Documentation CI workflow); the gen_wikidoc.sh eliomdoc->wiki script
and the README section are no longer used. The indexdoc menus are kept
(still consumed by doc/wodoc).

@Julow Julow left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The commit history on this branch is a complete mess. It needs to be totally reworked.

Comment thread CHANGES
signature; Ot.Icons.Make is now parameterised on
Eliom.Content.Html.T (the shared signature exposed by Eliom 13)
instead of an ad-hoc ICON_HTML signature.
* API regression: Ot.Form.password_toggle now takes the less

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you move this to an other PR ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

it is just a missing line in the changelog, not really in this PR 😬

Comment thread CHANGES
* Build system
** Use ocsigen-dune-rules (gen sub-command) to drive client/server
PPX rules (new opam dependency).
** CSS files are now installed via dune

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you move this into an other PR ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Same. This is already on master.

@balat

balat commented Jun 26, 2026

Copy link
Copy Markdown
Member Author

The commit history on this branch is a complete mess. It needs to be totally reworked.

Yes I agree.But it is WIP, not ready to review/merge. It was supposed to be merged step by step in master but master is frozen until the release, so we have the "eio-branch on Irmin" syndrom.

balat added 2 commits June 26, 2026 17:40
Match the version already used in practice (and pinned in ocsigen-start /
eliom). Reformat the few stragglers via dune fmt.
Eliom.Lib.Option (which re-exported the Ocsigen Server baselib Option module)
was removed in Server 8. Use the stdlib Option module instead (map/iter have
identical signatures).
@Julow

Julow commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

I see !

balat and others added 6 commits June 26, 2026 17:51
Adapt to Ocsigen Server 8 (+ ocamlformat 0.29)
Pin ocsigenserver#master, ocsipersist#master, eliom#deriving,
ocsigen-dune-rules#modernize and ocsigen-ppx-rpc#modernize so opam can
resolve the Server 8 chain (the default branches are still Server 7).
CI: pin the Server 8 dev branches of ocsigen deps
Since js_of_ocaml 6.4, Dom_html.inputElement##.files is a nullable Js.opt.
Guard the fileList access with Js.Opt.bind in picture_uploader. Bump the
js_of_ocaml lower bound to 6.4.0.
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