Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Refactor/depends on lodash#51

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
JaKXz merged 4 commits intoredux-utilities:masterfromtomchentw:refactor/depends-on-lodash
Feb 9, 2017
Merged

Refactor/depends on lodash#51

JaKXz merged 4 commits intoredux-utilities:masterfromtomchentw:refactor/depends-on-lodash
Feb 9, 2017

Conversation

@tomchentw
Copy link
Contributor

From#22 (comment)
@JaKXz instead of replacinglodash modular dependencies, would you consider switching dependencies tolodash main module and import from its entries? For example,

Before

importisPlainObjectfrom'lodash.isplainobject';importisStringfrom'lodash.isstring';importisSymbolfrom'lodash.issymbol';

After

importisPlainObjectfrom'lodash/isPlainObject';importisStringfrom'lodash/isString';importisSymbolfrom'lodash/isSymbol';

Thus, for those users who's also usinglodash in their user space, the bundler (rollup/webpack) could resolve these modules and create a smaller output.

@coveralls
Copy link

coveralls commentedFeb 8, 2017•
edited
Loading

Coverage Status

Coverage remained the same at 100.0% when pulling93824e8 on tomchentw:refactor/depends-on-lodash intof787b9d on acdlite:master.

Copy link
Contributor

@JaKXzJaKXz left a comment•
edited
Loading

Choose a reason for hiding this comment

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

@tomchentw thanks! though I still prefer what you originally suggested withlodash/isString etc.

Is there any reason you didn't use that [or am I missing something about howbabel-plugin-lodash works]?

@tomchentw
Copy link
ContributorAuthor

@JaKXz that's how it looks like when runningyarn run build:

screen shot 2017-02-08 at 12 11 43 pm

@JaKXz
Copy link
Contributor

@tomchentw oh ok, cool. Would destructuring those methods out oflodash also produce that result?

@tomchentw
Copy link
ContributorAuthor

@JaKXz I think so! Would you like switching to destructing format?

screen shot 2017-02-08 at 3 27 36 pm

JaKXz reacted with thumbs up emoji

@JaKXz
Copy link
Contributor

Yes, please :)

@tomchentw
Copy link
ContributorAuthor

@JaKXz updated!

@coveralls
Copy link

coveralls commentedFeb 9, 2017•
edited
Loading

Coverage Status

Coverage remained the same at 100.0% when pullinga299fde on tomchentw:refactor/depends-on-lodash into01e8fc3 on acdlite:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pullinga299fde on tomchentw:refactor/depends-on-lodash into01e8fc3 on acdlite:master.

@JaKXzJaKXz merged commit5eb76c5 intoredux-utilities:masterFeb 9, 2017
@JaKXz
Copy link
Contributor

JaKXz commentedFeb 9, 2017•
edited
Loading

Thanks@tomchentw! I'm going to bed now but I will release this by the end of the week when I have OSS cycles again :)

In the meanwhile please feel free to installflux-standard-action#master and give it a whirl.

tomchentw reacted with thumbs up emoji

@tomchentwtomchentw deleted the refactor/depends-on-lodash branchFebruary 9, 2017 07:03
@JaKXz
Copy link
Contributor

+ flux-standard-action@1.2.0 finally published! sorry for the delay!

@tomchentw
Copy link
ContributorAuthor

Awesome!

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

1 more reviewer

@JaKXzJaKXzJaKXz approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tomchentw@coveralls@JaKXz
[8]ページ先頭
©2009-2025 Movatter.jp