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

Make react-router-dom treeshakable #6465

Merged
merged 3 commits into from Nov 8, 2018
Merged

Make react-router-dom treeshakable #6465

merged 3 commits into from Nov 8, 2018

Conversation

TrySound
Copy link
Contributor

@TrySound TrySound commented Nov 7, 2018

Ref #6464

Here's the problem. Babel transpiles static class properties outside of class iife which blocks uglify/terser from this iife.

defaultProps will become a bad pattern soon since it's blocks treeshakability. For functions the problem is even more aggressive since it's not solvable these days.

I think the best we may do is use defaults in destructuring

const Component = ({ prop1 = 'a', prop2 = 'b' }) => {
}

With classes this is harder because defaults can't be propagated in all methods.

@billyjanitsch
Copy link
Contributor

billyjanitsch commented Nov 7, 2018

Thanks for looking into this @TrySound! Can you explain why this solution is preferred over simply not bundling the package into a single file for the ESM build? In other words, what is the advantage of bundling the file?

@TrySound
Copy link
Contributor Author

TrySound commented Nov 7, 2018

With single bundle it is easier to control size and output. It's bundled faster. More consistency in imports. Less import statements. Internals in splitted files are hidden which is the most important.

Treeshaking is machine work. Splitting imports with paths is user work.

@mjackson mjackson merged commit 4aefa2f into remix-run:master Nov 8, 2018
@mjackson
Copy link
Member

mjackson commented Nov 8, 2018

Thanks, @TrySound! I'll take a look at eliminating defaultProps from the react-router package.

mjackson added a commit that referenced this pull request Nov 8, 2018
@TrySound TrySound deleted the treeshakability branch November 8, 2018 08:02
@lock lock bot locked as resolved and limited conversation to collaborators Jan 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants