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.

Sign up for GitHub

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

Jump to bottom

Add Array#find polyfill to docs #150

Merged
merged 2 commits into from
Nov 23, 2018
Merged

Add Array#find polyfill to docs #150

merged 2 commits into from
Nov 23, 2018

Conversation

dalehurwitz
Copy link
Contributor

No description provided.

@dalehurwitz dalehurwitz force-pushed the feat-find-polyfill branch 2 times, most recently from 2123a88 to c357abe Compare November 22, 2018 22:21
dagoof
dagoof reviewed Nov 22, 2018
View reviewed changes
Copy link
Contributor

@dagoof dagoof left a comment

Choose a reason for hiding this comment

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

Not really sure what's right or wrong as we're all still learning gatsby as-we-go; but there's a mention here

gatsbyjs/gatsby#2177

about using onClientEntry as a place to load polyfills in this file. Not sure if that's the "right" way to do it

@jonathaningram
Copy link
Contributor

@dagoof @dalehurwitz link he's referring to is in comment gatsbyjs/gatsby#2177 (comment)

jonathaningram
jonathaningram reviewed Nov 22, 2018
View reviewed changes
Copy link
Contributor

@jonathaningram jonathaningram left a comment

Choose a reason for hiding this comment

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

Yeah @dalehurwitz I would say at least have a look into that technique and see if that's preferred? Also, did we need an update to package-lock.json?

@dalehurwitz
Copy link
Contributor Author

dalehurwitz commented Nov 23, 2018 •
edited
Loading

@jonathaningram @dagoof it works either way, but I've implemented onClientEntry to be inline with gatsby recommendations.

@jonathaningram package-lock.json was regenerated when I ran npm i. Generally has to do with dependencies of dependencies being updated.

jonathaningram
jonathaningram approved these changes Nov 23, 2018
View reviewed changes
Copy link
Contributor

@jonathaningram jonathaningram left a comment

Choose a reason for hiding this comment

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

Cool thanks.

@dalehurwitz dalehurwitz merged commit cafde49 into master Nov 23, 2018
@dalehurwitz dalehurwitz deleted the feat-find-polyfill branch November 23, 2018 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dagoof dagoof dagoof left review comments

@andrewoh531 andrewoh531 andrewoh531 approved these changes

@jonathaningram jonathaningram jonathaningram approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dalehurwitz @jonathaningram @dagoof @andrewoh531