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

Convert classes into tree-shakable ES6 modules #37

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sisou
Copy link
Member

@sisou sisou commented Jan 17, 2020

Convert all classes with only static methods into tree-shakable ES6 modules (as explained in #29 (comment)). Utilities can now be directly accessed as

import { isSafari } from '@nimiq/utils/dist/browser-detection/BrowserDetection';

which only bundles isSafari and tree-shakes all other methods, that were previously included (and thus bundled) in the BrowserDetection class.

(Changing the import path is a topic for another PR, because it would likely change the whole file structure.)

This change is backwards-compatible, as the main.js still exports whole namespaces with the original names. Please check if you agree.

The diff should be watched with whitespace turned off, since the indentation got reduced by one level.

@sisou sisou requested review from danimoh, mraveux and nibhar January 17, 2020 12:32
@sisou sisou self-assigned this Jan 17, 2020
Copy link
Member

@danimoh danimoh left a comment

Choose a reason for hiding this comment

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

👍

What changes did you have in mind for the file structure?
I think the source file structure doesn't have to change.
In dist, the module files could be moved out of their sub folders directly into dist after building.
Or do you even want to move the dist files directly into the packages root?
We could do this with a npm script that moves the files up, builds the package, then removes the files.

@sisou
Copy link
Member Author

sisou commented May 29, 2020

Yes, I would like to build the package in a way that does not include the dist folder, so that imports can be made directly from @nimiq/utils/Utf8Tools, which would be nice.

@sisou
Copy link
Member Author

sisou commented May 29, 2020

However, maybe this can simply be achieved by instead of packaging the dist directory, we instead package all files within the dist directory: files: 'dist/*'. (I have not tried this).

@sisou sisou force-pushed the soeren/tree-shakable-modules branch from bdabadc to d3e8659 Compare May 29, 2020 07:44
@sisou
Copy link
Member Author

sisou commented May 29, 2020

Additionally to the removal of the dist directory from the import path, the subfolders in the package also need to be removed. This can either be done by moving all *.js files up into dist/ during build, or by removing the subfolders in the source as well (which would be the big restructuring I mentioned).

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