-
Notifications
You must be signed in to change notification settings - Fork 145
add @babel/plugin-syntax-dynamic-import #756
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
base: main
Are you sure you want to change the base?
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 addresses a critical issue where JSON-LD dynamic imports were failing due to Babel transpilation issues. The fix involves two main approaches: configuring Babel to preserve dynamic imports in ESM builds and adding fallback logic to handle both ESM and CommonJS module formats.
Changes:
- Added
@babel/plugin-syntax-dynamic-importplugin and configured Babel to exclude dynamic import transformation in ESM builds - Updated
jsonldparser.jsto handle both ESM (jsonld.default) and CommonJS (jsonld) module formats - Added comprehensive RDFa parsing tests that exercise the JSON-LD parser functionality
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| .babelrc | Added dynamic import syntax plugin and excluded transform in ESM config |
| package.json | Added @babel/plugin-syntax-dynamic-import dependency |
| package-lock.json | Lock file updates for new dependency |
| src/jsonldparser.js | Added ESM/CommonJS compatibility with fallback logic |
| tests/unit/parse-test.js | Added RDFa test suite with JSON-LD serialization/parsing tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| [ | ||
| "@babel/preset-env", | ||
| { "modules": false } | ||
| { "exclude": ["proposal-dynamic-import"], |
Copilot
AI
Jan 15, 2026
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.
The exclude configuration uses "proposal-dynamic-import" but this may not match the actual plugin name used by @babel/preset-env version 7.28.x. The plugin was renamed from "proposal-dynamic-import" to "@babel/plugin-transform-dynamic-import" in earlier Babel 7 versions. You should verify this exclusion is working as intended by checking if dynamic imports are preserved in the ESM build output. The correct value might need to be "transform-dynamic-import" or "@babel/plugin-transform-dynamic-import" depending on the preset-env version.
| { "exclude": ["proposal-dynamic-import"], | |
| { "exclude": ["transform-dynamic-import"], |
| "@babel/plugin-proposal-class-properties", | ||
| "@babel/plugin-syntax-dynamic-import" |
Copilot
AI
Jan 15, 2026
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.
Adding @babel/plugin-syntax-dynamic-import may be unnecessary with @babel/preset-env version 7.28.x, as the preset already includes dynamic import syntax support. The plugin only enables parsing of dynamic import syntax and doesn't prevent transformation. To preserve dynamic imports in the output, you should focus on correctly excluding the transform plugin in the preset-env configuration (line 16 of .babelrc) rather than adding this syntax plugin. Consider removing this plugin and verifying that the exclude configuration alone solves the issue.
| "@babel/plugin-proposal-class-properties", | |
| "@babel/plugin-syntax-dynamic-import" | |
| "@babel/plugin-proposal-class-properties" |
tests/unit/parse-test.js
Outdated
| describe('html from test-suite', () => { | ||
| let store | ||
| let store1 | ||
| let ttlContent |
Copilot
AI
Jan 15, 2026
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.
The variable ttlContent is declared at the top but only assigned and used in the first test. It should be declared inside the specific test where it's used (line 556) to improve test isolation and clarity.
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.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
PR: Fix JSON-LD Dynamic Import Broken by Babel Transpilation
Problem
The
jsonldparser.jsuses dynamic import to load thejsonldmodule:This works perfectly in vanilla Node.js, but Babel's transpilation breaks it:
_interopRequireWildcardwrapper causes the module to load as an empty object{}TypeError: jsonld.flatten is not a function