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

Browser simulateScript - restore build #44

Merged
merged 8 commits into from
Dec 4, 2023

Conversation

ernest-nowacki
Copy link
Collaborator

https://smartcontract-it.atlassian.net/browse/FRONT-3851

Hey while debugging Firefox issues with simulate script I digged up the above ticket. During our initial implementation I've built simulateScript for frontend usage once and copied the output to our front end repo. While it got us moving now when we have package published I believe we could be using proper way.

There are few things happening here:

  1. I've noticed here webpack and all dependencies required to build frontend simulate script has been removed: a1c811d

Is that the intention? I could still see package.json script having build:browser script (which wasn't working anymore).

In this PR I'm restoring everything needed to build that simulate package. I noticed repo started using yarn so I adjusted PR for that (no more package-lock.json). All dependencies required to build front end shouldn't end up being shipped in the package as they are dev dependencies. If however it's still no longer the business of functions-toolkit package to build front end simulator I might try to extract building process somewhere else.

  1. There was build error reported by webpack when building simulator:

Screenshot 2023-11-02 at 12 41 06

That's because it couldn't handle the dynamic nature of allowed modules. I refactored this part of the code a bit to make sure there are no warnings during the build. This shall affect only browser simulator not deno version.

@ernest-nowacki ernest-nowacki changed the title Chore/build browser v2 Browser simulateScript - restore build Nov 2, 2023
@KuphJr
Copy link
Contributor

KuphJr commented Nov 8, 2023

Thanks @ernest-nowacki ! My only comment is we are now below our required test coverage threshold. We should either add tests, or at the very least ignore these files in Jest such that our workflows pass before merging.

@ernest-nowacki
Copy link
Collaborator Author

Hey @KuphJr thanks for checking the PR!

I had one error in unit tests + I noticed other tests are living under test directory. Fixing those issues get me back to 100% coverage on new addons 👍 .

@cl-sonarqube-production
Copy link

@ernest-nowacki ernest-nowacki merged commit 37de37f into main Dec 4, 2023
6 checks passed
@ernest-nowacki ernest-nowacki deleted the chore/build-browser-v2 branch December 4, 2023 11:54
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