Skip to content

group Jsx modules #7347

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aspeddro
Copy link
Contributor

@aspeddro aspeddro commented Mar 17, 2025

Creates a top-level module for Jsx. It contains the following submodules:

  • Jsx.DOMStyle
  • Jsx.Event
  • Jsx.DOM

Deprecate JsxDOM.res, JsxDOMStyle and JsxEvent

With these changes we can have a section in the sidebar menu for the Jsx module containing DOM, DOMStyle and Event

Copy link
Member

@cknitt cknitt left a comment

Choose a reason for hiding this comment

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

Thanks a lot @aspeddro!

This means, however, that we need to change rescript-react accordingly at some point.

@mununki do you see any issue with this?

runtime/Jsx.res Outdated
module Event = JsxEvent
module DOM = {
type style = DOMStyle.t
type domRef
Copy link
Member

Choose a reason for hiding this comment

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

I think it is problematic that the types defined here (other than style) are different from those in JsxDOM. They should all be aliases.

Copy link
Contributor Author

@aspeddro aspeddro Mar 25, 2025

Choose a reason for hiding this comment

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

JsxDOM.domProps.children is of type Jsx.element, defining a alias here creates a cyclic dependency Jsx.cmj -> JsxDOM.cmj -> Jsx.cmj

Unless we create a file Jsx_common.res to put the common values/types

@aspeddro aspeddro force-pushed the group-jsx-modules branch from 25f43c3 to 668ccb3 Compare March 25, 2025 13:01
Copy link

pkg-pr-new bot commented May 11, 2025

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript-lang/rescript@7347

@rescript/darwin-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-arm64@7347

@rescript/darwin-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-x64@7347

@rescript/linux-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-arm64@7347

@rescript/linux-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-x64@7347

@rescript/win32-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/win32-x64@7347

commit: dfb8221

runtime/Jsx.res Outdated
Comment on lines 25 to 40
// Define this as a private empty record so that the compiler does not
// unnecessarily add `Primitive_option.some` calls for optional props.
type element = private {}

@val external null: element = "null"

external float: float => element = "%identity"
external int: int => element = "%identity"
external string: string => element = "%identity"

external array: array<element> => element = "%identity"

type componentLike<'props, 'return> = 'props => 'return
type component<'props> = componentLike<'props, element>

/* this function exists to prepare for making `component` abstract */
external component: componentLike<'props, element> => component<'props> = "%identity"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to Jsx_common.res to avoid cyclic dependency.

@aspeddro aspeddro marked this pull request as ready for review May 11, 2025 20:38
@aspeddro aspeddro requested a review from cknitt May 11, 2025 20:40
@aspeddro aspeddro force-pushed the group-jsx-modules branch from dfb8221 to 1f70005 Compare May 31, 2025 16:52
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants