Repository navigation
Align various implementation details between hed-javascript and hed-typescript - #947
Merged
Merged
Conversation
- Split schema loading into AbstractHedSchemaLoader with Node.js and browser subclasses; the browser build gets HedSchemaBrowserLoader through the package.json browser field. - Load bundled schemas on demand: one ESM chunk per schema, CommonJS inline. - Move BidsDirectoryAccessor to its own module with a browser stand-in, replace node:path with string helpers, and drop the node-modules polyfill plugin. - Reject a library listed twice in one merge group when the version string is parsed (multipleVersionsOfSameLibrarySchema). - parseHedStrings returns null for the whole list if any string fails. - Spec tests read hed-tests schemas through a mock loader. - Align BIDS class shapes and helpers with hed-typescript; remove the unused getDir helper. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every path that reaches the Event constructor assigns defName first, so the initial null was never read (ESLint no-useless-assignment). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
VisLab
added
required-update
alignment
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The duplicate-library check rejects valid merges of distinct local schema files because it compares empty library values before loading their XML identities.
1 open issue
What changed in this PR
This PR aligns hed-javascript’s schema loading, BIDS handling, and utility behavior with hed-typescript, while separating browser and Node.js file access.
Changes:
- Introduces environment-specific schema loaders and on-demand schema chunks for the browser build, removing Node.js polyfills.
- Refactors BIDS accessors, parsing helpers, and validation internals.
- Updates duplicate-schema validation, parser return types, tests, and documentation.
| File | Description |
|---|---|
types/index.d.ts |
Updates accessor constructor and nullable parser results. |
tests/testHelpers/testUtilities.js |
Corrects the named issue import. |
tests/otherTests/webAccessor.spec.js |
Checks browser schema-loader selection. |
tests/otherTests/schemaParser.spec.js |
Updates the XML collection import. |
tests/otherTests/schemaLoader.spec.js |
Adds loader behavior and error tests. |
tests/otherTests/paths.spec.js |
Tests browser-compatible path helpers. |
tests/otherTests/parser.spec.js |
Tests whole-list failure results. |
tests/otherTests/datasetParser.spec.js |
Updates accessor imports and category assertions. |
tests/otherTests/dataset.spec.js |
Tests browser directory-access rejection. |
tests/otherTests/build.spec.js |
Checks browser bundle isolation and schema chunks. |
tests/jsonTestData/schemaBuildTests.data.js |
Updates duplicate-schema error cases. |
src/utils/types.js |
Adds JSON-object and string-record guards. |
src/utils/string.js |
Adds object-only JSON parsing. |
src/utils/paths.js |
Refactors path organization and removes Node.js dependency. |
src/utils/files.js |
Keeps local file reading; relocates HTTPS reading. |
src/utils/files.browser.js |
Removes obsolete browser file utilities. |
src/schema/xmlType.js |
Houses the XML collection and compatibility alias. |
src/schema/xmlCollection.js |
Removes the relocated collection module. |
src/schema/specs.js |
Rejects duplicate schema names during specification construction. |
src/schema/loader.js |
Implements the Node.js schema-loader subclass. |
src/schema/init.js |
Delegates schema construction to the loader. |
src/schema/config.js |
Loads bundled schemas through dynamic imports. |
src/schema/browserLoader.js |
Adds schema loading without local file access. |
src/schema/abstractLoader.js |
Centralizes schema loading and merge construction. |
src/schema/__mocks__/loader.js |
Adds test-schema lookup with bundled fallback. |
src/parser/parser.js |
Returns a null list when any string fails. |
src/parser/eventManager.js |
Leaves absent definition names undefined. |
src/issues/data.js |
Replaces the duplicate-schema diagnostic. |
src/bids/webAccessor.js |
Injects a browser-specific schema builder. |
src/bids/validator/validator.js |
Clarifies validator documentation. |
src/bids/validator/tsvValidator.js |
Consolidates issue collection and privatizes helpers. |
src/bids/validator/sidecarValidator.js |
Documents internal validation helpers. |
src/bids/types/tsv.js |
Extracts TSV parsing and HED-column initialization. |
src/bids/types/json.js |
Uses the shared JSON-object guard. |
src/bids/types/file.js |
Privatizes validator storage and adds toString(). |
src/bids/types/dataset.js |
Uses JSON-file objects and portable path parsing. |
src/bids/tsvParser.js |
Adds TSV typedefs and renames an internal helper. |
src/bids/schema.js |
Accepts an injectable schema loader. |
src/bids/index.js |
Updates accessor imports and removes the default export. |
src/bids/directoryAccessor.js |
Separates Node.js directory access. |
src/bids/directoryAccessor.browser.js |
Explicitly rejects browser directory access. |
src/bids/datasetParser.js |
Injects schema builders and isolates the base accessor. |
spec_tests/hedTests.spec.js |
Uses the shared mock schema loader. |
package.json |
Updates browser mappings and removes the polyfill dependency. |
package-lock.json |
Removes polyfill-related dependency entries. |
esbuild.mjs |
Enables browser schema chunks without Node.js polyfills. |
CHANGELOG.md |
Records alignment and behavior changes. |
.github/copilot-instructions.md |
Updates the documented module layout. |
Review effort: Balanced
A specification with only a local path has an empty library name until its file is read, so the parse-time duplicate check rejected a standard schema file merged with a library file. The parse-time check now skips local-path specifications, and buildSchemaObjects checks the names of the loaded schemas. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.