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: udpate deps #3520

Merged
merged 3 commits into from May 19, 2023
Merged

chore: udpate deps #3520

merged 3 commits into from May 19, 2023

Conversation

abdulsattar
Copy link
Contributor

Details

Update dependencies

Does this pull request introduce a breaking change?

  • ✅ No, it does not introduce a breaking change.

Does this pull request introduce an observable change?

  • ✅ No, it does not introduce an observable change.

GUS work item

"eslint-plugin-import": "^2.27.5",
"eslint-plugin-jest": "^27.1.7",
"fs-extra": "^11.1.1",
"glob": "^9.3.1",
"glob": "^10.2.5",
Copy link
Contributor

Choose a reason for hiding this comment

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

I checked, the breaking change is to remove the default export (isaacs/node-glob@26673b9), looks like we aren't using it anywhere anymore.

@@ -29,7 +29,7 @@
"@lwc/rollup-plugin-node-resolve-legacy": "npm:rollup-plugin-node-resolve@5.2.0",
"folder-hash": "4.0.4",
"markdown-table": "^3.0.3",
"tachometer": "0.5.10"
"tachometer": "0.7.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

We can't upgrade Tachometer. See the note above:

"Also note we are pinned to Tachometer 0.5.10 due to a breaking change in 0.6.0.",
"Breaking change: https://github.com/google/tachometer/issues/244",

Copy link
Contributor

@nolanlawson nolanlawson left a comment

Choose a reason for hiding this comment

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

We can't upgrade Tachometer, otherwise LGTM

['autofocus', new Set(['button', 'input', 'keygen', 'select', 'textarea'])],
['autoplay', new Set(['audio', 'video'])],
['checked', new Set(['command', 'input'])],
const BOOLEAN_ATTRIBUTES = /*@__PURE__@*/ new Map([
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needed because rollup started marking Map, Set etc. as impure. rollup/rollup#4955

We could mark Map, Set as pure in rollup.config.js, but that would be on the consumer to do, and if they, for some reason, don't do it, these are not tree-shaken.

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting! Great find. Looks like they're marking all Maps/Sets as impure as a temporary fix for rollup/rollup#4282 . In any case, we can keep the /*@__PURE__@*/ permanently to play it safe.

Copy link
Contributor

@nolanlawson nolanlawson left a comment

Choose a reason for hiding this comment

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

Looks like there is some kind of downstream breakage, otherwise LGTM

@abdulsattar
Copy link
Contributor Author

/nucleus ignore -m "broken check in downstream"

@abdulsattar abdulsattar merged commit ebedbc5 into master May 19, 2023
12 checks passed
@abdulsattar abdulsattar deleted the abdulsattar/update-deps branch May 19, 2023 21:05
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.

None yet

2 participants