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

✨ introduce script setup #4

Closed
wants to merge 10 commits into from
Closed

Conversation

joao-m-santos
Copy link
Contributor

WIP: A few npm packages need to be updated.

@joao-m-santos
Copy link
Contributor Author

@vivy27 could we replace flush-packages dependency with our own impl. of it?

@vivy27
Copy link
Contributor

vivy27 commented Oct 5, 2022

We could write our own flush, however, it's a dev dep, so doesn't add up to our bundle.

@joao-m-santos
Copy link
Contributor Author

We could write our own flush, however, it's a dev dep, so doesn't add up to our bundle.

True, but looking at this package src code makes me feel like we could just add it ourselves and not rely on an external package. In general, keeping the dependency list as small as possible is a good practice, since it's less prone to issues (security, package deletion, etc.)

@vivy27 vivy27 closed this Oct 5, 2022
@vivy27
Copy link
Contributor

vivy27 commented Oct 5, 2022

We could write our own flush, however, it's a dev dep, so doesn't add up to our bundle.

True, but looking at this package src code makes me feel like we could just add it ourselves and not rely on an external package. In general, keeping the dependency list as small as possible is a good practice, since it's less prone to issues (security, package deletion, etc.)

Yeah, That absolutely makes sense. I can add the function to flush promises.

@vivy27 vivy27 reopened this Oct 5, 2022
@vivy27 vivy27 force-pushed the script-setup-and-jest-fixes branch 3 times, most recently from 145250e to b367b3c Compare October 10, 2022 13:52
@vivy27 vivy27 changed the title Draft: ✨ introduce script setup ✨ introduce script setup Oct 17, 2022
@vivy27 vivy27 marked this pull request as ready for review October 17, 2022 12:39
@vivy27 vivy27 requested a review from a team as a code owner October 17, 2022 12:39
@vivy27 vivy27 force-pushed the script-setup-and-jest-fixes branch 3 times, most recently from 93adb5f to 0c4b0f6 Compare October 24, 2022 10:02
@vivy27 vivy27 force-pushed the script-setup-and-jest-fixes branch from 0c4b0f6 to 2ad8710 Compare October 26, 2022 14:06
@vivy27 vivy27 force-pushed the script-setup-and-jest-fixes branch from 2ad8710 to 1fed2b0 Compare November 3, 2022 12:23
@netlify
Copy link

netlify bot commented Nov 28, 2022

Deploy Preview for adyen-lume ready!

Name Link
🔨 Latest commit 5e27bb3
🔍 Latest deploy log https://app.netlify.com/sites/adyen-lume/deploys/638f0e175afca30009534159
😎 Deploy Preview https://deploy-preview-4--adyen-lume.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@vivy27 vivy27 force-pushed the script-setup-and-jest-fixes branch from 9485021 to 5e27bb3 Compare December 6, 2022 09:40
@vivy27
Copy link
Contributor

vivy27 commented Jan 2, 2023

Closing this MR as this branch is quite outdated. Therefore, created a new MR #121

@vivy27 vivy27 closed this Jan 2, 2023
@joao-m-santos joao-m-santos deleted the script-setup-and-jest-fixes branch February 1, 2023 10:56
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