-
Notifications
You must be signed in to change notification settings - Fork 464
move Lazy module to Stdlib #7399
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR moves the Lazy module from its old location into Stdlib by renaming all references and updating corresponding API calls in tests and library modules.
- Update imports from "rescript/lib/es6/Lazy.js" to "rescript/lib/es6/Stdlib_Lazy.js".
- Replace lazy creation and evaluation calls (e.g. from_fun → make, force → get) in tests and source files.
- Update artifacts and Stdlib exports to reflect the new module location.
Reviewed Changes
Copilot reviewed 13 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
tests/tests/src/test_primitive.mjs | Changed lazy API calls to use Stdlib_Lazy.get. |
tests/tests/src/recursive_module.mjs | Replaced Lazy.from_fun/force calls with Stdlib_Lazy.make/get. |
tests/tests/src/mpr_6033_test.mjs | Updated lazy function calls to use Stdlib_Lazy.get. |
tests/tests/src/lazy_test.mjs | Adjusted lazy API calls (including isEvaluated, make, get) accordingly. |
tests/tests/src/lazy_demo.mjs | Migration of lazy API usage from Lazy to Stdlib_Lazy. |
tests/tests/src/gpr_3697_test.mjs | Switched lazy API calls, though one instance still uses from_fun. |
packages/artifacts.txt | Removed old Lazy module references and added new Stdlib_Lazy entries. |
lib/js/Stdlib_Lazy.js & lib/es6/Stdlib_Lazy.js | Added exports for make, get, and isEvaluated from the new Lazy implementation. |
lib/js/Stdlib.js & lib/es6/Stdlib.js | Updated exports to include Lazy, though its initialization appears incomplete. |
CHANGELOG.md | Updated changelog to document the module move. |
Files not reviewed (8)
- runtime/Lazy.resi: Language not supported
- runtime/Stdlib.res: Language not supported
- runtime/Stdlib_Lazy.res: Language not supported
- runtime/Stdlib_Lazy.resi: Language not supported
- tests/tests/src/lazy_demo.res: Language not supported
- tests/tests/src/lazy_test.res: Language not supported
- tests/tests/src/mpr_6033_test.res: Language not supported
- tests/tests/src/recursive_module.res: Language not supported
Comments suppressed due to low confidence (2)
lib/js/Stdlib.js:52
- The exported 'Lazy' variable is declared but not initialized. Consider assigning it the new Stdlib_Lazy module (e.g. via require('./Stdlib_Lazy.js')) or removing the export if it is no longer needed.
let Lazy;
lib/es6/Stdlib.js:52
- The 'Lazy' variable is declared without being assigned a value before being exported. It should be properly initialized with the new Stdlib_Lazy module or removed if obsolete.
let Lazy;
_0: Stdlib_Lazy.from_fun(fix) | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stdlib_Lazy.from_fun is used here, but the new API exports the lazy function creation as 'make'. Replace 'Stdlib_Lazy.from_fun' with 'Stdlib_Lazy.make' to avoid potential runtime errors.
_0: Stdlib_Lazy.from_fun(fix) | |
}; | |
_0: Stdlib_Lazy.make(fix) |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point from Copilot, but this test file is a bit weird anyway, as it doesn't actually run any code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes those are the tests I don't fully understand ^^
from_fun
is still valid though and only deprecated for now, do you want me to correct it @cknitt ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! Thanks a lot!
// FIXME: | ||
// This exists for compatibility reason. | ||
// Move this into Pervasives or Core | ||
|
||
type t<'a> = lazy_t<'a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not in scope of this PR, but once all "lazy" magic is removed from the compiler (see also #6960), this could just be an abstract type here (instead of a predefined type lazy_t
from predef.ml).
_0: Stdlib_Lazy.from_fun(fix) | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point from Copilot, but this test file is a bit weird anyway, as it doesn't actually run any code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't seem necessary to keep deprecated methods here. They were introduced in v12-alpha.1. =)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@namenu they were moved in v12-alpha.1 but they were already in a Lazy
module.
example in v11
No description provided.