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

refactor(console): remove babel macro #1675

Merged
merged 1 commit into from
Sep 13, 2024
Merged

refactor(console): remove babel macro #1675

merged 1 commit into from
Sep 13, 2024

Conversation

matyasf
Copy link
Collaborator

@matyasf matyasf commented Sep 12, 2024

It was only used to insert the #__PURE__ annotation, but it was buggy and it got inserted twice, both in the wrong place (it must be before a function call, we put it before an export statement).
https://rollupjs.org/configuration-options/#pure
https://webpack.js.org/guides/tree-shaking/#mark-a-function-call-as-side-effect-free

TEST PLAN:
Compare the built binaries with the previous version. There should be no differences in other packages. You can do this online here: https://npmdiff.dev/%40instructure%2Fui-tabs/10.2.1/10.2.2-pr-snapshot-1726213252671 (the SNAPSHOT version is from a manual release in this branch https://github.com/instructure/instructure-ui/actions/runs/10845048343 )

Copy link

github-actions bot commented Sep 12, 2024

PR Preview Action v1.4.7
🚀 Deployed preview to https://instructure.github.io/instructure-ui/pr-preview/pr-1675/
on branch gh-pages at 2024-09-13 15:25 UTC

@@ -174,7 +174,7 @@ function getWebEnvConfig(opts) {
},
useBuiltIns: 'entry',
// this version has to match the version in package.json
corejs: '3.26.1',
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a separate small fix, doesnt seem to change anything

@matyasf matyasf self-assigned this Sep 13, 2024
Copy link
Contributor

@balzss balzss left a comment

Choose a reason for hiding this comment

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

nice job getting to the bottom of this

@matyasf matyasf force-pushed the remove_babel_macro branch 2 times, most recently from 80e66e0 to 9ba7c62 Compare September 13, 2024 15:13
… in Vite/Rollup

It was in the wrong place and 2 times. The whole macro was used just to add these buggy annotations
@matyasf matyasf merged commit 48e78bb into master Sep 13, 2024
6 of 7 checks passed
@matyasf matyasf deleted the remove_babel_macro branch September 13, 2024 15:16
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.

3 participants