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.

Sign up for GitHub

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

Jump to bottom

%todo extension #6713

Merged
merged 8 commits into from
Apr 4, 2024
Merged

%todo extension #6713

merged 8 commits into from
Apr 4, 2024

Conversation

zth
Copy link
Collaborator

@zth zth commented Apr 3, 2024 •
edited
Loading

This adds a %todo extension, for which the compiler will warn (but not fail) when encountered:

let implementMeLater = (): string => %todo("This should return a string eventually.")

let x = implementMeLater()

Js.log(x->Js.String2.includes("x"))

Gives:
image(I've shamelessly taken the warning text from a screenshot of the same functionality in Gleam. We should change that if we want to merge this)

And compiles to the following JS:

// Generated by ReScript, PLEASE EDIT WITH CARE
'use strict';

var Js_exn = require("rescript/lib/js/js_exn.js");

function implementMeLater(param) {
  return Js_exn.raiseError("tst.res:1:37-42 - Todo: This should return a string eventually.");
}

var x = Js_exn.raiseError("tst.res:1:37-42 - Todo: This should return a string eventually.");

console.log(x.includes("x"));

exports.implementMeLater = implementMeLater;
exports.x = x;
/* x Not a pure module */

%todo doesn't need to have a payload string unless you want it to:
image

// Generated by ReScript, PLEASE EDIT WITH CARE
'use strict';

var Js_exn = require("rescript/lib/js/js_exn.js");

function implementMeLater(param) {
  return Js_exn.raiseError("tst.res:1:37-42 - Todo");
}

var x = Js_exn.raiseError("tst.res:1:37-42 - Todo");

console.log(x.includes("x"));

exports.implementMeLater = implementMeLater;
exports.x = x;
/* x Not a pure module */

Notice that the source ReScript file name + location is also included in the runtime error.

You can also easily run the compiler with this warning producing an error instead (as to make sure no %todo makes it into actual prod builds, or similar): rescript -warn-error +110

The %todo node itself is transformed into Js.Exn.raiseError(todoMessage).

@zth zth force-pushed the poc-todo-extension branch from f0b1228 to 08fd41d Compare April 4, 2024 07:06
@zth
Copy link
Collaborator Author

zth commented Apr 4, 2024

I've updated the PoC to throw errors instead of using Obj.magic(), and also include the source ReScript file name + location in the runtime errors thrown.

@tsnobip
Copy link
Contributor

tsnobip commented Apr 4, 2024

I think this is a great feature, I would definitely use it! This is the best of both worlds, you can prototype quickly and still let the compiler track all the places you have to complete.

@mununki
Copy link
Member

mununki commented Apr 4, 2024

Good feature! I'd love to use it once released.

@zth zth changed the title PoC: %todo extension %todo extension Apr 4, 2024
@zth zth marked this pull request as ready for review April 4, 2024 09:33
@zth zth requested a review from cristianoc April 4, 2024 09:33
@zth zth merged commit 104fe9f into 11.0_release Apr 4, 2024
14 checks passed
@zth zth deleted the poc-todo-extension branch April 4, 2024 09:54
@tsnobip tsnobip mentioned this pull request Apr 4, 2024
Closed
@aspeddro aspeddro mentioned this pull request Apr 6, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mununki mununki mununki left review comments

@aspeddro aspeddro aspeddro left review comments

@cristianoc cristianoc cristianoc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@zth @tsnobip @mununki @cristianoc @aspeddro