Skip to content
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

Use JSON imports rather than a custom Babel transform #18563

Open
nicolo-ribaudo opened this issue Aug 5, 2024 · 1 comment
Open

Use JSON imports rather than a custom Babel transform #18563

nicolo-ribaudo opened this issue Aug 5, 2024 · 1 comment
Labels

Comments

@nicolo-ribaudo
Copy link
Contributor

The Babel plugin has a custom JSON transform that replaces PDFJSDev.json("$ROOT/some/path.json") with the contents of some/path.json:

case "json":
if (!t.isStringLiteral(arg)) {
throw new Error("Path to JSON is not provided");
}
let jsonPath = arg.value;
if (jsonPath.startsWith(ROOT_PREFIX)) {
jsonPath = joinPaths(
ctx.rootPath,
jsonPath.substring(ROOT_PREFIX.length)
);
}
return JSON.parse(fs.readFileSync(jsonPath, "utf8"));
}

This is only used once, in the Chromium extension code:

pdf.js/web/chromecom.js

Lines 197 to 199 in c60c0d1

const i18nFileAccessLabel = PDFJSDev.json(
"$ROOT/web/chrome-i18n-allow-access-to-file-urls.json"
)[chrome.i18n.getUILanguage?.()];

There is a Stage 3 proposal that would allow directly importing JSON files:

import i18nFileAccessLabels from "./chrome-i18n-allow-access-to-file-urls.json" with { type: "json" };

This feature is already supported by default by Node.js, Chrome, Safari and Webpack. Given that the code where we use PDFJSDev.json is bundled with webpack, this syntax is safe to use and we can remove the custom PDFJSDev.json feature.

However, ESLint doesn't support this syntax yet because it's only a Stage 3 proposal and not part of the standard yet (it will hopefully become part of the standard in October). There are two options:

  • wait until the first ESLint minor release after October, which will support this syntax by default
  • use @babel/eslint-parser to let ESLint parse this syntax
@Snuffleupagus
Copy link
Collaborator

Snuffleupagus commented Aug 5, 2024

  • wait until the first ESLint minor release after October, which will support this syntax by default

Given just how little that pre-processor feature is being used, this option seems overall less disruptive than making changes to our linting setup itself. (Especially since we should, at some point, switch to ESLint 9; which is being tracked in issue #17928.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants