Skip to content

Migrate codebase to use ESM#7730

Merged
whymarrh merged 5 commits intoMetaMask:developfrom
whymarrh:esm
Jan 9, 2020
Merged

Migrate codebase to use ESM#7730
whymarrh merged 5 commits intoMetaMask:developfrom
whymarrh:esm

Conversation

@whymarrh
Copy link
Copy Markdown
Contributor

@whymarrh whymarrh commented Dec 27, 2019

This PR migrates the majority of the JavaScript in the codebase from CommonJS (i.e. require) to ECMAScript Modules (i.e. import/export).

This has a few benefits:

  1. Consistency—the background scripts and the UI are now more similar in style
  2. Static analysis—all imports and exports must be defined at "compile" time instead of dynamically at runtime, allow for better static analysis and tooling (think tree-shaking, dead code elimination)
  3. Easier TypeScript migration—this is a step towards the import syntax that TypeScript uses as TypeScript modules are aligned with ECMAScript Modules[1] and thus this will allows us an easier migration to TypeScript in the future

On that last point: I factored this task out of work I was doing to get the codebase to compile under TypeScript.

This PR is large, admittedly, and will conflict with many things. I'll be updating it as conflicts arise but hopefully we can merge this relatively quickly.

@whymarrh whymarrh force-pushed the esm branch 12 times, most recently from cca2fc4 to 1090263 Compare January 7, 2020 19:47
@whymarrh whymarrh marked this pull request as ready for review January 7, 2020 20:09
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

Looks good!

I didn't go through this one with a fine-toothed comb, but I did look at every file, and it does seem to be mostly just the straightforward import/export conversions. The actionConstants change is probably the biggest thing aside from that, and I suspect ESLint would be able to catch those sort of mistakes pretty well (e.g. references to top-level exports that are now under that object).

@whymarrh whymarrh merged commit 92971d3 into MetaMask:develop Jan 9, 2020
@whymarrh whymarrh deleted the esm branch January 9, 2020 03:35
yqrashawn pushed a commit to yqrashawn/conflux-portal that referenced this pull request Jan 14, 2020
* Update eslint-plugin-import version

* Convert JS files to use ESM

* Update ESLint rules to check imports

* Fix test:unit:global command env

* Cleanup mock-dev script
yqrashawn pushed a commit to Conflux-Chain/conflux-portal that referenced this pull request Jan 15, 2020
* Update eslint-plugin-import version

* Convert JS files to use ESM

* Update ESLint rules to check imports

* Fix test:unit:global command env

* Cleanup mock-dev script
Gudahtt added a commit that referenced this pull request Mar 11, 2020
The `createRetryTransaction` was accidentally removed from the
`gas-modal-page-container` component during a refactor in #7730.
Attempting to retry a transaction since that change has resulted in a
UI crash.
@Gudahtt Gudahtt mentioned this pull request Mar 11, 2020
Gudahtt added a commit that referenced this pull request Mar 11, 2020
The `createRetryTransaction` was accidentally removed from the
`gas-modal-page-container` component during a refactor in #7730.
Attempting to retry a transaction since that change has resulted in a
UI crash.
github-merge-queue bot pushed a commit that referenced this pull request Mar 5, 2025
#30661)

## **Description**

@davidmurdoch requested this feature here:
#30440 (review)

Also adds to the VSCode GitLens settings. If a Cursor user could help
with the Cursor settings, that would be much appreciated.

We should discuss:

- The inclusion of #17092, as I'm undecided about it
- Whether it's appropriate to automatically execute `git config
blame.ignoreRevsFile .git-blame-ignore-revs` in `postinstall`. It writes
to the local `.git/config` file in your `metamask-extension` folder, so
it's only changing that one folder.

Command to get commits with over 200 file changes
```
git log --pretty=format:"%H %s" --shortstat | awk '{if ($1 ~ /^[0-9]+$/) {num = $1 + 0; if (num > 200 && current_hash !~ /Revert/) print num " " current_hash} else current_hash = $0}' | sort -nr
```

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/30661?quickstart=1)

## **Related issues**

David Murdoch request:
#30440 (review)

## **Ignored PRs**

- #6304 
- #7730 
- #8023 
- #8056 
- #8595 
- #9239 
- #9274 
- #10358
- #10655
- #10911
- #17092
- #22639
- #22531
- #30440

<!--## **Manual testing steps**
## **Screenshots/Recordings**
## **Pre-merge author checklist**
## **Pre-merge reviewer checklist**-->

---------

Co-authored-by: David Murdoch <187813+davidmurdoch@users.noreply.github.com>
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.

4 participants