Add some docs to stblib Symbol #7336

jderochervlk · 2025-03-13T02:02:53Z

This adds a .resi file for Stdlib_Symbol with documentation for most of the common uses for Symbols. I added a description and toString to the bindings.

A good chunk of what Symbols are used for aren't really applicable or possible with ReScript, such as the static properties. I'm not sure if there is value in including those bindings in the stdlib, but for now I just omitted documentation.

tsnobip

thanks for your PR! I think you have to add Stdlib_Symbol.cmti/resi to packages/artifacts.txt to make the tests pass.

While you're there, I'd also convert the documentations to docstring tests, that'd make it even more useful!

tsnobip · 2025-03-14T11:19:34Z

runtime/Stdlib_Symbol.resi

+Console.log(Symbol.keyFor(globalSym))
+// Expected output: "sym1"


Make those a test instead of just documenting it, it'd be even more useful!

Suggested change

Console.log(Symbol.keyFor(globalSym))

// Expected output: "sym1"

Symbol.keyFor(globalSym)->assetEqual("sym1")

tsnobip

two typos to fix and we should be good to go :)

tsnobip · 2025-03-14T15:45:35Z

runtime/Stdlib_Symbol.resi

+## Examples
+
+```rescript
+Symbol.make("sym1")->assetEqual("sym1")


my bad it's assertEqual

Suggested change

Symbol.make("sym1")->assetEqual("sym1")

Symbol.make("sym1")->assertEqual("sym1")

tsnobip · 2025-03-14T15:45:42Z

runtime/Stdlib_Symbol.resi

+## Examples
+
+```rescript
+Symbol.getFor("sym1")->assetEqual(Symbol.getFor("sym1"))


tsnobip · 2025-03-14T15:52:57Z

runtime/Stdlib_Symbol.resi

+```
+*/
+@val
+external getFor: string => t = "Symbol.for"


you also forgot to update the interface file, and while we're there, I'd also use @scope for these bindings, which is cleaner IMO:

Suggested change

external getFor: string => t = "Symbol.for"

@scope("Symbol")

external getFor: string => option<t> = "for"

tsnobip · 2025-03-14T15:54:34Z

@jderochervlk tests fail, did you try to run them locally?

make lib
make tests

jderochervlk · 2025-03-14T16:26:54Z

make tests

I'm trying to get things to run locally, but the setup docs seem to leave me in a place where things don't work.

➜  rescript git:(symbol) make lib      
cargo build --manifest-path rewatch/Cargo.toml
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.04s
cp rewatch/target/debug/rewatch rewatch
./scripts/copyExes.js -rewatch
dune build
./scripts/copyExes.js -compiler           
./scripts/buildRuntime.sh
Cleaning... 941 files.
>>>> Start compiling
Dependency Finished
rescript: [678/678] Belt.cmj
>>>> Finish compiling 1030 mseconds
./scripts/prebuilt.js
Version check: package.json: 12.0 vs ABI: 12.0
➜  rescript git:(symbol) make tests    
make: Nothing to be done for 'tests'.
➜  rescript git:(symbol)

tsnobip · 2025-03-14T16:43:17Z

make tests

I'm trying to get things to run locally, but the setup docs seem to leave me in a place where things don't work.

➜  rescript git:(symbol) make lib      
cargo build --manifest-path rewatch/Cargo.toml
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.04s
cp rewatch/target/debug/rewatch rewatch
./scripts/copyExes.js -rewatch
dune build
./scripts/copyExes.js -compiler           
./scripts/buildRuntime.sh
Cleaning... 941 files.
>>>> Start compiling
Dependency Finished
rescript: [678/678] Belt.cmj
>>>> Finish compiling 1030 mseconds
./scripts/prebuilt.js
Version check: package.json: 12.0 vs ABI: 12.0
➜  rescript git:(symbol) make tests    
make: Nothing to be done for 'tests'.
➜  rescript git:(symbol)

sorry @jderochervlk I meant

make test

jderochervlk · 2025-03-14T16:47:26Z

sorry @jderochervlk I meant
make test

I'm getting a permissions issue.

➜  rescript git:(symbol) make test
cargo build --manifest-path rewatch/Cargo.toml
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.07s
cp rewatch/target/debug/rewatch rewatch
./scripts/copyExes.js -rewatch
dune build
./scripts/copyExes.js -compiler           
./scripts/buildRuntime.sh
Cleaning... 941 files.
>>>> Start compiling
Dependency Finished
rescript: [678/678] Belt.cmj
>>>> Finish compiling 1538 mseconds
./scripts/prebuilt.js
Version check: package.json: 12.0 vs ABI: 12.0
node scripts/test.js -all
Checking OCaml code formatting...
✅ OCaml code formatting ok.             
Checking ReScript code formatting...
✅ ReScript code formatting ok.
Biome format check

> rescript@12.0.0-alpha.10 checkFormat
> biome format --changed --no-errors-on-unmatched .

sh: 1: biome: Permission denied
node:internal/errors:865
  const err = new Error(message);
              ^

Error: Command failed: bash scripts/format_check.sh
    at checkExecSyncError (node:child_process:890:11)
    at Object.execSync (node:child_process:962:15)
    at runTests (/home/josh/Dev/rescript/scripts/test.js:48:8)
    at Object.<anonymous> (/home/josh/Dev/rescript/scripts/test.js:186:1)
    at Module._compile (node:internal/modules/cjs/loader:1364:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1422:10)
    at Module.load (node:internal/modules/cjs/loader:1203:32)
    at Module._load (node:internal/modules/cjs/loader:1019:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:128:12) {
  status: 126,
  signal: null,
  output: [ null, null, null ],
  pid: 25486,
  stdout: null,
  stderr: null
}

Node.js v18.20.7
make: *** [Makefile:33: test] Error 1

tsnobip · 2025-03-14T17:14:00Z

@jderochervlk try deleting node_modules and install again.

jderochervlk · 2025-03-14T19:22:14Z

@jderochervlk try deleting node_modules and install again.

Weird, that did the trick, thanks!

jderochervlk · 2025-03-14T20:10:38Z

Tests are now passing locally.

tsnobip

Looks good to me now :)

Add some docs to stblib Symbol

398d0f5

jderochervlk mentioned this pull request Mar 13, 2025

Documentation and docstrings rescript-lang/rescript-core#25

Open

49 tasks

tsnobip requested changes Mar 14, 2025

View reviewed changes

jderochervlk added 4 commits March 14, 2025 08:19

add to artifacts

42cdf74

start switching to assert

5520d1d

check

0326321

all asserts

e5734fa

jderochervlk requested a review from tsnobip March 14, 2025 15:30

tsnobip requested changes Mar 14, 2025

View reviewed changes

tsnobip reviewed Mar 14, 2025

View reviewed changes

pr feedback

eb1e28c

jderochervlk requested a review from tsnobip March 14, 2025 16:31

jderochervlk added 3 commits March 14, 2025 15:38

fixing tests

370d85a

work

25113a7

tests passing

f6debf9

tsnobip approved these changes Mar 15, 2025

View reviewed changes

tsnobip merged commit 67b3870 into rescript-lang:master Mar 17, 2025
20 checks passed

jderochervlk deleted the symbol branch March 17, 2025 14:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add some docs to stblib Symbol #7336

Add some docs to stblib Symbol #7336

jderochervlk commented Mar 13, 2025

tsnobip left a comment

tsnobip Mar 14, 2025

tsnobip left a comment

tsnobip Mar 14, 2025

jderochervlk Mar 14, 2025

tsnobip Mar 14, 2025

jderochervlk Mar 14, 2025

tsnobip Mar 14, 2025

jderochervlk Mar 14, 2025

tsnobip commented Mar 14, 2025

jderochervlk commented Mar 14, 2025

tsnobip commented Mar 14, 2025

jderochervlk commented Mar 14, 2025

tsnobip commented Mar 14, 2025

jderochervlk commented Mar 14, 2025

jderochervlk commented Mar 14, 2025

tsnobip left a comment

		Console.log(Symbol.keyFor(globalSym))
		// Expected output: "sym1"

	Console.log(Symbol.keyFor(globalSym))
	// Expected output: "sym1"
	Symbol.keyFor(globalSym)->assetEqual("sym1")

	Symbol.make("sym1")->assetEqual("sym1")
	Symbol.make("sym1")->assertEqual("sym1")

	external getFor: string => t = "Symbol.for"
	@scope("Symbol")
	external getFor: string => option<t> = "for"

Add some docs to stblib Symbol #7336

Add some docs to stblib Symbol #7336

Conversation

jderochervlk commented Mar 13, 2025

tsnobip left a comment

Choose a reason for hiding this comment

tsnobip Mar 14, 2025

Choose a reason for hiding this comment

tsnobip left a comment

Choose a reason for hiding this comment

tsnobip Mar 14, 2025

Choose a reason for hiding this comment

jderochervlk Mar 14, 2025

Choose a reason for hiding this comment

tsnobip Mar 14, 2025

Choose a reason for hiding this comment

jderochervlk Mar 14, 2025

Choose a reason for hiding this comment

tsnobip Mar 14, 2025

Choose a reason for hiding this comment

jderochervlk Mar 14, 2025

Choose a reason for hiding this comment

tsnobip commented Mar 14, 2025

jderochervlk commented Mar 14, 2025

tsnobip commented Mar 14, 2025

jderochervlk commented Mar 14, 2025

tsnobip commented Mar 14, 2025

jderochervlk commented Mar 14, 2025

jderochervlk commented Mar 14, 2025

tsnobip left a comment

Choose a reason for hiding this comment