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

Changes to file structure to also support using Mithril globally #36

Open
felix-roehrich opened this issue Mar 23, 2019 · 6 comments
Open

Comments

@felix-roehrich
Copy link

felix-roehrich commented Mar 23, 2019

I propose changing the file structure, so this type declaration, also supports global usage.

Benefits:

  • Only one package to maintain instead of two
  • Users do not need to create an extra types.d.ts file
  • Works out of the box, so potential beginning programmers are not scared away by having to write the types.d.ts file, and newcomers are not annoyed that it does not work [out of the box] (this is also the reason why I almost dropped Mithril; for example the Vnode in "mithril-global" has no default, which is annoying if you are working with explicit types)

Changes:

  • moved the content from index.d.ts to mithril.d.ts and created new index.d.ts
  • moved the declare functions at the beginning to the Static interface (so they are not exposed)
  • some small changes to the export statements at the end

For the new usage you can take a look at this minimal example

P.S. I am using GoLand, so some feedback on other IDEs would be appreciated, though I expect it to work anywhere.

Edit: The namespace can be renamed Mithril -> m, so you can still write m.Vnode instead of Mithril.Vnode.

@spacejack
Copy link
Collaborator

We would have liked to make a unified global/module definition file however the way Mithril's stream package works in a script/global context makes this problematic. When the stream script is added to a page, it attaches itself to the global m object. So you have m.stream. If the global mithril script is not present (i.e., the user did not include mithri.js), then stream creates its own (empty) global m object to attach stream to. This is why we have separate global/module typedefs.

@felix-roehrich
Copy link
Author

felix-roehrich commented Mar 25, 2019

I understand that if you use require you have to declare both mithril and mithril/stream,
but in global context you expect it as part of m. So, if you simply do not want stream as part of m if you use require then this example should work.

@spacejack
Copy link
Collaborator

Stream is not automatically included with mithril.js, you have to add the stream.js script to your html. So the core mithril types and stream types are separate.

The module types are easy, it's the global ones (which conditionally tack-on stream to m) that take extra work to accommodate.

@felix-roehrich
Copy link
Author

felix-roehrich commented Mar 25, 2019

I know that Stream is not automatically included. I do not know if you took a look at my example but I cannot understand your problem. In my example the typedefs are still separate (though there is a reference to the stream types), but in mithril-global Stream is also part of the namespace by default. So maybe you just draw the line (on what means separate) different from me.

Edit: I think it is possible to make unified typedef. It does not have to be like in my example (I also got some other ideas) but as long as I do not understand what your problem exactly is, I cannot offer other suggestions.

@spacejack
Copy link
Collaborator

There are 3 separate cases that need to work (in a global context):

  1. Mithril only (no Stream types included)
  2. Stream only (no Mithril types, only Stream - attached to a m global)
  3. Mithril and Stream both on the m object

@felix-roehrich
Copy link
Author

felix-roehrich commented Mar 25, 2019

Your description and the current API seem to imply that different structure (which would affect Mithril package itself) is more appropriate (because Core and Stream are/seem of equal value):

  • mithril/
    • stream/ <- Stream (no Mithril types)
    • core/ <- Mithril core (no Stream types)
    • index.d.ts <- Mithril with Stream

If somebody only wants the stream/core component they can require it as mithril/stream or mithril/core; for both components mithril. (I will address the global version below.)

With this approach, you would clearly separate Core and Stream components and could get the require/global version in one unified typedef. As mentioned above this seems to be the most reasonable move, though you might disagree.

Finally, and this is probably annoying for you (as it is for me), I simply do not understand your reasoning: I understand that there are three separate cases that need to work independently from one another, but the current mithril-global typedef does not distinguish between these cases.
If you want the stream/core typedefs for require and global completely separate, then you might want to serve the core component as mithril and the stream component as mithril-stream (for package and typedef).
With this approach you can use augmentation, such that, if only mithril is installed (via npm), the m global contains only the core, if only mithril-stream is installed, m only contains the stream components, and if both are installed, it contains both components.
(Needless to say people using the CommonJS approach require the respective package.)

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

No branches or pull requests

2 participants