Conversation
The Plugin type now returns error (changed in jsonic/go v0.1.16). Update Expr to return nil and tolerate the new Use return type at call sites. Known regression: TestSpecUnarySuffix* (4 suites) and TestSpecParenImplicitMap fail against jsonic/go >= v0.1.13 due to behavior changes in the underlying lexer/parser. These tests pass on v0.1.12. Further investigation needed to align the expr plugin with the new jsonic behavior. https://claude.ai/code/session_017W6amvYxE2ZmaTp3obEVS2
- Replace @hapi/code with a small node:assert-based expect shim in test/spec-util.ts. Normalize null-prototype objects so deep equality matches @hapi/code's behavior. - Drop @jsonic/doc from devDependencies and the doc script. - Go fix: the expr rule's Close alts lacked a true catch-all. In jsonic/go v0.1.13+, ParseAlts raises jsonic/unexpected when alts are defined but none match. Mirror the TS "expr,expr-end" fallback so the expr rule can end on any non-expression token (e.g. ZZ after "1!" or a space-separated key inside parens). All 727 Go subtests now pass against jsonic/go v0.1.18. https://claude.ai/code/session_017W6amvYxE2ZmaTp3obEVS2
Runs go build ./... and go test ./... in the go/ module across ubuntu/windows/macos with Go 1.24 and stable. https://claude.ai/code/session_017W6amvYxE2ZmaTp3obEVS2
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cab67b5cf6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const jsonic_1 = require("jsonic"); | ||
| const __1 = require(".."); | ||
| const spec_util_1 = require("./spec-util"); |
There was a problem hiding this comment.
dist-test/expr.test.js now requires ./spec-util, but this commit does not add a corresponding dist-test/spec-util.js artifact (the dist-test/ tree still only contains the two *.test.js files). Since npm test runs dist-test/**/*.test.js directly, a clean checkout that runs tests without a rebuild will fail at module load time with Cannot find module './spec-util' before any tests execute.
Useful? React with 👍 / 👎.
Four compiled test artifacts (expr.test.js/.map, ternary.test.js/.map) were tracked even though dist-test/ is gitignored. They went stale when test sources changed, and the prior commit broke clean-checkout runs: the tracked expr.test.js required ./spec-util, which wasn't tracked, so npm test failed at module load before any build. - Remove the four stale tracked artifacts from git. - Add pretest: "npm run build" so npm test always runs against fresh output. CI already runs build before test; this makes local clean checkouts work too. https://claude.ai/code/session_017W6amvYxE2ZmaTp3obEVS2
2 participants
https://claude.ai/code/session_017W6amvYxE2ZmaTp3obEVS2