Skip to content

Ensure highlight.js builds only the files that are imported #724

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

Conversation

dobromir-hristov
Copy link
Contributor

@dobromir-hristov dobromir-hristov commented Aug 1, 2023

Bug/issue #, if applicable: 113252221

Summary

Fixes and issue with webpack pulling in more highlight.js files than it should + fixes the node modules path

Dependencies

NA

Testing

Steps:

  1. Ensure when building, we build the correct set of language files
  2. Ensure the new import path does not cause errors when referenced by extending docc-render

Checklist

Make sure you check off the following items. If they cannot be completed, provide a reason.

  • Added tests
  • Ran npm test, and it succeeded
  • Updated documentation if necessary

Copy link
Contributor

@mportiz08 mportiz08 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Just have a few questions mostly for my own curiosity.

@@ -90,7 +90,7 @@ async function importHighlightFileForLanguage(language) {
languageFile = await import(
// See bug https://github.com/webpack/webpack/issues/13865
/* webpackChunkName: "highlight-js-[request]" */
`@/../node_modules/highlight.js/lib/languages/${file}`
`highlight-js-alias/lib/languages/${file}`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be using the new alias in all places where we import files from this package or is this the only import that needed to be explicitly changed to use the alias? Just curious.

I'm guessing just this one since it's the only one using the @.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah. the alias is just for here, because there is a webpack bug that we cannot dynamically import those lang files, so it needs an absolute path.

@dobromir-hristov
Copy link
Contributor Author

@swift-ci test

@dobromir-hristov dobromir-hristov merged commit 6f9f638 into swiftlang:main Aug 2, 2023
@dobromir-hristov dobromir-hristov deleted the dhristov/fix-highlight-js-bunlding branch August 2, 2023 06:57
mportiz08 added a commit to mportiz08/swift-docc-render that referenced this pull request Sep 12, 2023
This is a regression mistakenly introduced with [swiftlang#724][0] that prevents
non-custom syntax languages (everything except swift/markdown) from
being highlighted as expected due to an `import` call that is throwing
an exception.

To fix it, we can simply append the `.js` file extension to ensure that
the import continues to work as expected.

[0]: swiftlang#724
mportiz08 added a commit that referenced this pull request Sep 13, 2023
This is a regression mistakenly introduced with [#724][0] that prevents
non-custom syntax languages (everything except swift/markdown) from
being highlighted as expected due to an `import` call that is throwing
an exception.

To fix it, we can simply append the `.js` file extension to ensure that
the import continues to work as expected.

Resolves: rdar://115169709

[0]: #724
mportiz08 added a commit to mportiz08/swift-docc-render that referenced this pull request Oct 17, 2023
This is a regression mistakenly introduced with [swiftlang#724][0] that prevents
non-custom syntax languages (everything except swift/markdown) from
being highlighted as expected due to an `import` call that is throwing
an exception.

To fix it, we can simply append the `.js` file extension to ensure that
the import continues to work as expected.

Resolves: rdar://115169709

[0]: swiftlang#724
mportiz08 added a commit that referenced this pull request Oct 17, 2023
This is a regression mistakenly introduced with [#724][0] that prevents
non-custom syntax languages (everything except swift/markdown) from
being highlighted as expected due to an `import` call that is throwing
an exception.

To fix it, we can simply append the `.js` file extension to ensure that
the import continues to work as expected.

Resolves: rdar://115169709

[0]: #724
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants