Skip to content

Refactor: PR #1 feedback #4

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

Merged
merged 15 commits into from
Jun 11, 2018
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,14 @@ indent_size = 2
trim_trailing_whitespace = true
Copy link
Member

Choose a reason for hiding this comment

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

Maybe a comment at the top to say that Prettier will inherit from here? E.g. https://github.com/unsplash/web-ops/blob/7352b543ae110247b005cb60226c6adf213401b1/.editorconfig#L1

insert_final_newline = true

[patches/**.patch]
trim_trailing_whitespace = false

[.vscode/settings.json]
indent_size = 4

[*.snap]
trim_trailing_whitespace = false

# Git is sensitive to whitespace in diff files
# https://stackoverflow.com/questions/50258565/git-editing-hunks-fails-when-file-has-other-hunks/50275053#50275053
[*.diff]
trim_trailing_whitespace = false

[*.{ts,tsx}]
# https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length
max_line_length = 100
22 changes: 0 additions & 22 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,31 +1,9 @@
# OS X
.DS_Store

# Logs
logs
*.log
npm-debug.log*

# Runtime data
pids
*.pid
*.seed

# node-waf configuration
.lock-wscript

# Compiled binary addons (http://nodejs.org/api/addons.html)
build/Release

# Dependency directory
# https://docs.npmjs.com/misc/faq#should-i-check-my-node-modules-folder-into-git
node_modules

# Optional npm cache directory
.npm

# Optional REPL history
.node_repl_history

# typescript dist
dist/
4 changes: 4 additions & 0 deletions .npmignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
tsconfig.json
src
.vscode
.github
.editorconfig
.prettierrc
Copy link
Member

Choose a reason for hiding this comment

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

Instead of .npmignore you can specify a files whitelist inside package.json. Depends what you prefer!

2 changes: 0 additions & 2 deletions .prettierrc
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
{
"trailingComma": "all",
"printWidth": 100,
"tabWidth": 2,
"singleQuote": true
}
13 changes: 10 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,26 @@
"main": "dist/index.js",
"types": "dist/index.d.ts",
"dependencies": {
"react": "^16.4.0"
"react": "^16.4.0",
"@types/react": "^16.3.17"
},
"devDependencies": {
"@types/react": "^16.3.17",
"typescript": "^2.9.1"
},
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
"compile": "tsc",
"prepublishOnly": "npm run compile"
},
"repository": {
"type": "git",
"url": "git+https://github.com/unsplash/react-progressive-enhancement.git"
},
"contributors": [
{
"name": "Sami Jaber",
"email": "[email protected]"
}
],
"license": "MIT",
"bugs": {
"url": "https://github.com/unsplash/react-progressive-enhancement/issues"
Expand Down
13 changes: 7 additions & 6 deletions src/consumer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@ import * as React from 'react';
import { Consumer, ProgressiveEnhancementProp } from './context';
import { ObjectOmit, getDisplayName } from './helpers';

export const withIsEnhanced = function<OwnProps extends ProgressiveEnhancementProp>(
export const withIsEnhanced = <OwnProps extends ProgressiveEnhancementProp>(
ComposedComponent: React.ComponentType<OwnProps>,
) {
type OwnPropsWithoutProgressiveEnhancementProp = ObjectOmit<OwnProps, ProgressiveEnhancementProp>;

const ComponentWithIsEnhanced: React.SFC<OwnPropsWithoutProgressiveEnhancementProp> = props => (
) => {
type ComponentWithIsEnhancedType = React.SFC<ObjectOmit<OwnProps, ProgressiveEnhancementProp>>;
const ComponentWithIsEnhanced: ComponentWithIsEnhancedType = props => (
<Consumer>
{({ isEnhanced }) => <ComposedComponent isEnhanced={isEnhanced} {...props} />}
</Consumer>
Expand All @@ -19,7 +18,9 @@ export const withIsEnhanced = function<OwnProps extends ProgressiveEnhancementPr
return ComponentWithIsEnhanced;
};

export const progressivelyEnhance = function<Props>(ComposedComponent: React.ComponentType<Props>) {
export const progressivelyEnhance = <Props extends {}>(
ComposedComponent: React.ComponentType<Props>,
) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To use generics with arrow functions in .tsx files, we have to do make the generic extend another type. This is so that TypeScript knows it's looking at a generic and not a JSX tag. Otherwise, the syntax will be ambiguous to the compiler 😕

See microsoft/TypeScript#4922

const ProgressivelyEnhance: React.SFC<Props> = props => (
<Consumer>
{({ isEnhanced }) => (isEnhanced ? <ComposedComponent {...props} /> : null)}
Expand Down
2 changes: 1 addition & 1 deletion src/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { ComponentType } from 'react';

export const getDisplayName = (ComposedComponent: ComponentType<any>) =>
ComposedComponent.displayName || 'Component';
ComposedComponent.displayName !== undefined ? ComposedComponent.displayName : 'Component';

export type Omit<T, K> = Pick<T, Exclude<keyof T, K>>;
export type ObjectOmit<T extends K, K> = Omit<T, keyof K>;
4 changes: 2 additions & 2 deletions src/provider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import { ProgressiveEnhancementProp, Provider, defaultValue } from './context';

import { getDisplayName } from './helpers';

export const enableProgressiveEnhancementsOnMount = function<Props>(
export const enableProgressiveEnhancementsOnMount = <Props extends {}>(
ComposedComponent: React.ComponentType<Props>,
): React.ComponentType<Props> {
): React.ComponentType<Props> => {
class ProgressiveEnhancementProvider extends React.Component<Props, ProgressiveEnhancementProp> {
static displayName = `ProgressiveEnhancementProvider(${getDisplayName(ComposedComponent)})`;

Expand Down