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

refactor(v2): convert @docusaurus/plugin-ideal-image to TypeScript #2011

Merged
merged 5 commits into from
Dec 1, 2019

Conversation

gimdongwoo
Copy link
Contributor

@gimdongwoo gimdongwoo commented Nov 17, 2019

refactor @docusaurus/plugin-ideal-image

Motivation

I want to refactor every plugin to typescript. It helps consistency and maintainability.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Compiling and testing are passed.

Related PRs

#1997

refactor @docusaurus/plugin-ideal-image
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Nov 17, 2019
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Nov 17, 2019

Deploy preview for docusaurus-2 ready!

Built with commit 882bad9

https://deploy-preview-2011--docusaurus-2.netlify.com

@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Nov 17, 2019

Deploy preview for docusaurus-preview ready!

Built with commit 882bad9

https://deploy-preview-2011--docusaurus-preview.netlify.com

Copy link
Contributor

@endiliey endiliey left a comment

Choose a reason for hiding this comment

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

Actually this broke the plugin itself :(
https://deploy-preview-2011--docusaurus-2.netlify.com/showcase/

It's a permanent low quality placeholder now

Copy link
Contributor

@yangshun yangshun left a comment

Choose a reason for hiding this comment

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

Thank you so much @gimdongwoo! Please have a look at the comments

@@ -0,0 +1,21 @@
declare module '@endiliey/react-ideal-image' {
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to add the copyright header

/**
 * Copyright (c) 2017-present, Facebook, Inc.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */

@@ -4,10 +4,13 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
import {LoadContext} from '@docusaurus/types';
Copy link
Contributor

Choose a reason for hiding this comment

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

Leave an empty line after the copyright header.


module.exports = function(context, options) {
export = function(_context: LoadContext, options: PluginOptions) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be export default

@yangshun yangshun merged commit 1047339 into facebook:master Dec 1, 2019
@gimdongwoo
Copy link
Contributor Author

@yangshun I’m really sorry. I am too busy recent 2 weeks.
I will be more attention for the next time.
Thank you so much.

@yangshun
Copy link
Contributor

yangshun commented Dec 1, 2019

No problem, thanks for starting this, the fixes required were really minor. Thank you!!! 🎉

@endiliey
Copy link
Contributor

endiliey commented Dec 1, 2019

OOps this PR broke something
image

@yangshun
Copy link
Contributor

yangshun commented Dec 1, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants