-
-
Notifications
You must be signed in to change notification settings - Fork 268
Add @metamask/build-utils package
#3577
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
Conversation
|
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
e128233 to
0a78bb7
Compare
- Fix title in readme - Add default for shouldLintTransformFiles in package readme - Simply ESLint mock in test
0a78bb7 to
204d142
Compare
- Emphasize that stylistic rules should probably be disabled for the ESLint instance. - Address parameter order inconsistency with removeFencedCode.
Jonathansoufer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job! LGTM!
| "@types/eslint": "^8.44.7", | ||
| "@types/jest": "^27.4.1", | ||
| "deepmerge": "^4.2.2", | ||
| "eslint": "^8.44.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that this is in devDependencies, but lintTransformedFile takes an argument that's of type ESLint. So, two questions:
- Where does this type come from — does it come from this package or
@types/eslint? Do we need both or just one? ESLintwill show up in the type definition file forsrc/transforms/utils.ts, so anyone who uses this library in a TypeScript will need that type in order to compile their code. We've encountered problems with this type of thing before and the solution has been to move the library in question todependencies(e.g., Moveeth-queryback from devDeps to deps #3578). What are your thoughts on moving eithereslintor@types/eslinttodependencies?
…cy (#3588) ## Explanation Following the recommendation of @mcmire, converts `@types/eslint` from a dev to production dependency of `@metamask/build-utils`. The reason: > ESLint will show up in the type definition file for `src/transforms/utils.ts`, so anyone who uses this library in a TypeScript will need that type in order to compile their code. ## References - #3577 (comment) ## Changelog N/A (package unreleased) ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate
## **Description** In MetaMask/metamask-mobile#7734, code fence support was added to MetaMask Mobile by essentially copypasting the implementation from the extension. This PR replaces the local code fence implementation with a version that was extracted into its own package. The external package implements code fence parsing and removal, as well as linting transformed files. The consumer is responsible for integrating the fence parser into the given build system, and configuring the ESLint instance used for linting. Consequently, the following remains in this repository after extracting the fence parsers: - `RemoveFencedCodeTransform`, a transform stream compatible with Browserify - A utility for configuring and initializing a singleton ESLint instance The PR is deceptively large; most changes are due to renaming existing fences. For reviewers, the relevant files are: - `development/build/transforms/*` - `lavamoat/build-system/policy.json` ## **Related issues** - MetaMask/core#3577 - MetaMask/metamask-mobile#7972 ## **Manual testing steps** 1. Create different builds of the extension and verify that they are working and include the correct features --------- Co-authored-by: MetaMask Bot <metamaskbot@users.noreply.github.com>
#7972) ## **Description** In #7734, code fence support was added to MetaMask Mobile by essentially copypasting the implementation from the extension. This PR replaces the local code fence implementation with a version that was extracted into its own package. The external package implements code fence parsing and removal, as well as linting transformed files. The consumer is responsible for integrating the fence parser into the given build system, and configuring the ESLint instance used for linting. Consequently, only the Metro transform and the ESLint instance initialization remain in this repository. ## **Related issues** - MetaMask/core#3577 - MetaMask/metamask-extension#22033 ## **Manual testing steps** 1. Check that different build types are built correctly
Explanation
In MetaMask/metamask-mobile#7734, code fence support was added to MetaMask Mobile by essentially copypasting the implementation from the extension. This PR extracts the shared logic from the extension and mobile implementations of code fencing into a new package,
@metamask/build-utils, which can be imported into the respective clients. A draft integration in the extension can be found in MetaMask/metamask-extension#22033, which uses local copies of the built version of the new package.@metamask/build-utilscurrently implements the shared logic for code fence parsing and removal,, as well as linting transformed files. The consumer is responsible for integrating the fence parser into the given build system, and configuring the ESLint instance used for linting.Noteworthy changes / decisions made:
ONLY_INCLUDE_INhas been renamed toONLY_INCLUDE_IF, which is more in line with its actual usage.removeFencedCodeandlintTransformedFilefunctions was made for two reasons primarily:References
@metamask/build-utilsmetamask-extension#22033Changelog
@metamask/build-utilsChecklist