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

(chore) Deprecate the requireLanguage API #2844

Merged
merged 10 commits into from
Nov 13, 2020

Conversation

joshgoebel
Copy link
Member

@joshgoebel joshgoebel commented Nov 11, 2020

Changes

  • Entire library now no longer uses any run-time dependencies (no grammar will break because another is missing)
  • Add docs for fixMarkup deprecation
  • Add docs for requireLanguage deprecation
  • Remove Requires: meta-data used only for run-time dependencies.
  • Gzip size is only ~230 bytes larger than before
  • Fix auto-detect tests now that the equivalency between cpp and arduino is no longer hidden by the load order.
  • adds supersetOf to resolve conflicts between very similar languages (C++, Arduino). The base language wins in the case of a tie. ie arduino is now a supersetOf cpp. (closes Add baseLangage to language grammars #2275)

Note: Requires: still stays overall as it's used to clue the build process about "loose" (non-breaking) sublanguage dependencies between languages. This dependency is purposely loose and therefore still a thing.

Checklist

  • Added markup tests, or they don't apply here because...
  • Updated the changelog at CHANGES.md
  • Added myself to AUTHORS.txt, under Contributors

Copy link
Member

@allejo allejo left a comment

Choose a reason for hiding this comment

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

Looks fine to me

Comment on lines +801 to +802
console.warn("requireLanguage is deprecated and will be removed entirely in the future.");
console.warn("Please see https://github.com/highlightjs/highlight.js/pull/2844");
Copy link
Member

Choose a reason for hiding this comment

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

Should there be some sort of check here for checking whether highlight.js is running a production environment?

Copy link
Member Author

Choose a reason for hiding this comment

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

The idea is that everyone can see it. It only shows up in console so it’s not disruptive. And after this PR it’s also immediately actionable so it’s not something that people have to wait till version 11. Someone can see it immediately make changes to that code base to make it go away.

I’m pretty sure this is consistent with the other deprecation notices.

Copy link
Member Author

Choose a reason for hiding this comment

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

I do certainly wish there were some better way to handle this but like I can’t really think of what it would be too of my head.

Copy link
Member

Choose a reason for hiding this comment

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

I don't really feel strongly about this either way. My initial thought was someone using a plug-in in their CMS that had a bundled version of highlight.js or something similar and fixing the warning was beyond their control until the plug-in maintainer updated it.

Closest thing I can think of is webpack's support for process.env.node_env === 'production' but I can't say I've ever bothered looking for a browser-only solution other than having *.production.min.js and *.development.js files

Copy link
Member Author

Choose a reason for hiding this comment

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

There is minified or not (perhaps could have a flag based on the build type) but there is no guarantee one is using minified in production and un-minified for development work. Unless you're hacking on the library you're probably just using the minified version everywhere.

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.

Add baseLangage to language grammars
2 participants