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

v4.7.1 broke node <= 6 #4127

Closed
1 task done
ljharb opened this issue Aug 11, 2023 · 11 comments · Fixed by #4274
Closed
1 task done

v4.7.1 broke node <= 6 #4127

ljharb opened this issue Aug 11, 2023 · 11 comments · Fixed by #4274
Labels
fix Bug fixes

Comments

@ljharb
Copy link
Contributor

ljharb commented Aug 11, 2023

Product

axe-core

Product Version

v4.7.1

Latest Version

  • I have tested the issue with the latest version of the product

Issue Description

Expectation

Be requireable in node < 6.

Actual

Crashes, because Object.values was added in node 7.

How to Reproduce

Run require('axe-core') in node 6.

Additional context

I suggest using https://npmjs.com/object.values until you're ready to release a major version that drops support for node 4-6 (please fix v4 for node <= 6 before doing that). I'd be happy to submit a PR.

@ljharb ljharb added the ungroomed Ticket needs a maintainer to prioritize and label label Aug 11, 2023
ljharb pushed a commit to jsx-eslint/eslint-plugin-jsx-a11y that referenced this issue Aug 11, 2023
ljharb pushed a commit to jsx-eslint/eslint-plugin-jsx-a11y that referenced this issue Aug 11, 2023
@straker
Copy link
Contributor

straker commented Aug 14, 2023

Thanks for the issue. Looks like the problem is in colorjs.io which we brought in to fix #4007.

@straker straker added bug and removed ungroomed Ticket needs a maintainer to prioritize and label labels Aug 14, 2023
@ljharb
Copy link
Contributor Author

ljharb commented Aug 14, 2023

I’m confused why it would have ended up bundled into the required entrypoint node if it’s a separate package, but that explains why i couldn’t find the source code in this repo.

@LeaVerou, would you accept a PR to improve engine compat for colorjs.io?

@LeaVerou
Copy link

I’m confused why it would have ended up bundled into the required entrypoint node if it’s a separate package, but that explains why i couldn’t find the source code in this repo.

@LeaVerou, would you accept a PR to improve engine compat for colorjs.io?

Sure, though if the solution is to bundle a polyfill, I'd rather do it in a separate bundle, so that the one targeting modern environments can stay lean. We can discuss over at the colorjs repo 😊

@ljharb
Copy link
Contributor Author

ljharb commented Sep 7, 2023

Looks like v4.8.0 also broke node < 16 by using Object.hasOwn somewhere.

@straker
Copy link
Contributor

straker commented Sep 7, 2023

@ljharb Thanks for pointing that out. Looks like that was our fault and added in #4060

@ljharb
Copy link
Contributor Author

ljharb commented Sep 7, 2023

@straker thanks for the pointer; filed #4152 to fix that.

@straker
Copy link
Contributor

straker commented Oct 9, 2023

Closed via v4.8.2

@straker straker closed this as completed Oct 9, 2023
@ljharb
Copy link
Contributor Author

ljharb commented Oct 9, 2023

@straker i don't think this was fully fixed, since colorjs.io hasn't been updated to have a polyfill yet. #4127 (comment) was a separate but related issue.

@ljharb
Copy link
Contributor Author

ljharb commented Oct 19, 2023

@straker can this please be reopened?

@straker
Copy link
Contributor

straker commented Oct 19, 2023

Yep sorry, I closed because we fixed the Object.hasOwn problem and forgot this was talking about Object.values. We should probably bring in an Object.values polyfill to fix the color.js issue.

@straker straker reopened this Oct 19, 2023
@straker straker added this to the Axe-core 4.9 milestone Oct 19, 2023
@ljharb
Copy link
Contributor Author

ljharb commented Oct 19, 2023

Unfortunately if it's bundled inside color.js, i think it might need to be fixed there, but I haven't had a chance to dig into it yet.

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

Successfully merging a pull request may close this issue.

4 participants