Monorepo: remove default exports #2018

gabrocheleau · 2022-07-05T18:04:54Z

This PR removes all instances of default exports within the monorepo. It also adds the 'import/no-default-export' linting rule that will prevent the addition of new default exports.

Resolves #2013

codecov · 2022-07-05T18:07:49Z

Codecov Report

Merging #2018 (edfd022) into master (5b1da6a) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

Flag	Coverage Δ
block	`84.30% <100.00%> (ø)`
blockchain	`80.06% <100.00%> (ø)`
client	`78.34% <100.00%> (ø)`
common	`95.03% <100.00%> (ø)`
devp2p	`82.71% <100.00%> (+0.06%)`	⬆️
ethash	`90.81% <100.00%> (ø)`
evm	`40.32% <100.00%> (-0.03%)`	⬇️
rlp	`90.47% <100.00%> (-0.08%)`	⬇️
statemanager	`84.55% <100.00%> (ø)`
trie	`73.72% <100.00%> (-0.49%)`	⬇️
tx	`92.17% <100.00%> (ø)`
util	`87.03% <100.00%> (ø)`
vm	`78.58% <100.00%> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

holgerd77

Just went through for a first round, some things to address, generally this looks really really great though, thanks for all the work here!! 😄 👍

Please be really careful on the branch update with the VM/EVM README files since I put a lot of work in these rewriting the text and coming up with an adopted structure. So please do not use the merge function or so, this will close to for sure not work. I guess best rather is here to just copy over the versions from master and the manually re-apply the path adoptions, can't imagine that there is another chance to get this done right.

Also: I would like to merge this in early here, since changes are so vast, this will likely get conflicting with other work pretty quickly (I e.g. already postponed additional README work on the other libs today due to some fear of getting into the same situation as in VM/EVM again).

So, in case this isn't complete yet, it would be good if we can in doubt rather fix the current state and then merge here and do the work on top in favor of having these open for several more days. But for sure also totally depends on a) what's open and b) what are your free times to tackle. So this is rather just a general note and very much needs to be adopted to the situation.

holgerd77 · 2022-07-07T09:50:49Z

packages/block/CHANGELOG.md

+    // Provide your block data here or use default values
+  },
+  { common }
+)


Ah, please do not change code examples from CHANGELOG files from the older (non-beta) library versions. The CHANGELOG examples are always "tied" to the respective versions, otherwise it would make not much sense.

Sorry, this might get a bit laborious, since this is actually present in all/most CHANGELOG files.

Yeah I left the changes in the .md files in a separate commit so that they're easily removable, but I'm unsure why a lot of this sort of formatting was automatically done, I assumed it was project-configured formatting that only got triggered now that changes were made to these files? Anyhow will revert most of these, if we want to improve .md formatting we can always do it in another PR.

holgerd77 · 2022-07-07T09:55:52Z

packages/blockchain/README.md

-| `blockchain:clique` | Clique operations like updating the vote and/or signer list  |
+| Logger              | Description                                                 |
+| ------------------- | ----------------------------------------------------------- |
+| `blockchain:clique` | Clique operations like updating the vote and/or signer list |


I guess I am not such a fan of these corrections, since this also renders correctly with the short version, so this just leads to slightly more work, since one now always has to adopt the length when rewriting the description.

On the plus side I see that this is somewhat more readable in the raw format.

Not sure, maybe there are also other opinions from the team and we can find a rather common ground? Also do not want to impose my personal preferences here (and some similar other locations).

holgerd77 · 2022-07-07T10:00:00Z

packages/evm/CHANGELOG.md


 const common = new Common({ chain: Chain.Mainnet, hardfork: Hardfork.Merge })
 const vm = new VM({ common })
 vm._common.isActivatedEIP(4399) // true
 ```

 - [EIP-4399](https://eips.ethereum.org/EIPS/eip-4399) Support: Supplant DIFFICULTY opcode with PREVRANDAO, PR [#1565](https://
-github.com/ethereumjs/ethereumjs-monorepo/pull/1565)
+  github.com/ethereumjs/ethereumjs-monorepo/pull/1565)


Ah, no, these kind of format corrections are not correct, this simply destroys the correct rendering and draws the URL apart, see the rendered version of this file.

Please make sure that you haven't updated anywhere else in a similar fashion.

holgerd77 · 2022-07-07T10:01:22Z

packages/evm/CHANGELOG.md

@@ -439,7 +439,7 @@ See PR [#1198](https://github.com/ethereumjs/ethereumjs-monorepo/pull/1198).

 **Features**

- Added `receipt` to `RunTxResult`, moved the tx receipt generation logic from `VM.runBlock()` to `VM.runTx()` (`generateTxReceipt()` and receipt exports in `runBlock` are now marked as *deprecated*), PR [#1185](https://github.com/ethereumjs/ethereumjs-monorepo/pull/1185)
+- Added `receipt` to `RunTxResult`, moved the tx receipt generation logic from `VM.runBlock()` to `VM.runTx()` (`generateTxReceipt()` and receipt exports in `runBlock` are now marked as _deprecated_), PR [#1185](https://github.com/ethereumjs/ethereumjs-monorepo/pull/1185)


Just curious, since these corrections are all over the place: isn't *deprecated* (e.g.) correct markdown? 🤔

holgerd77 · 2022-07-07T10:03:09Z

packages/evm/src/precompiles/04-identity.ts

@@ -1,7 +1,7 @@
 import { PrecompileInput } from './types'
 import { OOGResult, ExecResult } from '../evm'

-export default function (opts: PrecompileInput): ExecResult {
+export function precompile04(opts: PrecompileInput): ExecResult {


Yeah, definitely a better name than my pc suggestion. 🙂

holgerd77

Love it (to speak with Tim Beiko 😋), also thanks for addressing this so quickly then I can continue with the README updates today! ❤️

Also totally love the linting rule and the generality of this - thanks for suggesting - this is now just such a pure simplification and a general we-are-done-with-it thing - really with no substantial downsides - always great to have these structural simplifications in the whole monorepo where one has the feeling that things just get easier to handle.

Will merge.

gabrocheleau force-pushed the monorepo/remove-default-exports branch 2 times, most recently from 33db4c5 to 6e08e3b Compare July 5, 2022 18:52

gabrocheleau force-pushed the monorepo/remove-default-exports branch 2 times, most recently from 9078f91 to 1b99dbb Compare July 6, 2022 19:09

holgerd77 requested changes Jul 7, 2022

View reviewed changes

gabrocheleau added 3 commits July 7, 2022 13:20

Monorepo: remove default exports and adjust imports in non-test files

49682a0

Monorepo: adjust removal of default exports in test files

3c4c031

Monorepo: add no-default-export eslint rule

2c95530

gabrocheleau force-pushed the monorepo/remove-default-exports branch from 1b99dbb to 2c95530 Compare July 7, 2022 17:23

Monorepo: update README with non-default named imports

edfd022

gabrocheleau requested a review from holgerd77 July 7, 2022 20:04

gabrocheleau marked this pull request as ready for review July 7, 2022 20:04

holgerd77 added PR state: needs review and removed PR state: WIP labels Jul 8, 2022

holgerd77 approved these changes Jul 8, 2022

View reviewed changes

holgerd77 merged commit b30f914 into master Jul 8, 2022

holgerd77 deleted the monorepo/remove-default-exports branch July 8, 2022 08:43

holgerd77 mentioned this pull request Sep 20, 2022

Uncaught TypeError: Common is not a constructor #978

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Monorepo: remove default exports #2018

Monorepo: remove default exports #2018

gabrocheleau commented Jul 5, 2022 •

edited

Loading

codecov bot commented Jul 5, 2022 •

edited

Loading

holgerd77 left a comment

holgerd77 Jul 7, 2022

gabrocheleau Jul 7, 2022

holgerd77 Jul 7, 2022

holgerd77 Jul 7, 2022

holgerd77 Jul 7, 2022

holgerd77 Jul 7, 2022

holgerd77 Jul 7, 2022

holgerd77 Jul 7, 2022

holgerd77 left a comment

Monorepo: remove default exports #2018

Monorepo: remove default exports #2018

Conversation

gabrocheleau commented Jul 5, 2022 • edited Loading

codecov bot commented Jul 5, 2022 • edited Loading

Codecov Report

holgerd77 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

holgerd77 left a comment

Choose a reason for hiding this comment

gabrocheleau commented Jul 5, 2022 •

edited

Loading

codecov bot commented Jul 5, 2022 •

edited

Loading