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

chore: upgrades minor and patch deps #420

Merged
merged 7 commits into from
Dec 21, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 3 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"typescript.tsdk": "node_modules/typescript/lib"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, we configured it here. I had similar one in my user settings.

}
9 changes: 2 additions & 7 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,14 @@
"version-nightly": "bash scripts/should-nightly-run.sh && npm version prerelease --no-git-tag-version --preid=$(git rev-parse HEAD)",
"postversion": "bash scripts/push-if-git.sh",
"bundlesize": "yarn build && yarn build:dead-code-elimination && size-limit",
"postinstall": "npx yarn-deduplicate"
"postinstall": "npx yarn-deduplicate && yarn --ignore-scripts"
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we ignoring postinstall script for dependencies? Any specific reason? 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

only for postinstall - this is to stabalize the yarn lock after de-duping (and so it doesnt result in an infinite loop)

unfortunately there can be instances where running yarn after yarn de-dupe will result in the yarn lock file updating again

},
"devDependencies": {
"@babel/preset-env": "^7.11.0",
"@babel/preset-react": "^7.10.4",
"@babel/preset-typescript": "^7.10.4",
"@compiled/react": "*",
"@compiled/jest": "*",
"@compiled/react": "*",
"@emotion/core": "^10.1.1",
"@size-limit/preset-small-lib": "^4.4.5",
"@storybook/addons": "^5.2.8",
Expand Down Expand Up @@ -80,11 +80,6 @@
"playground/*",
"examples/packages/*"
],
"resolutions": {
"typescript": "3.7.x",
"@types/node": "13.11.1",
"@testing-library/dom": "7.2.2"
},
"size-limit": [
{
"path": "./packages/react/dist/browser/runtime.js",
Expand Down
4 changes: 2 additions & 2 deletions packages/babel-plugin/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@
"@compiled/css": "0.5.1",
"@compiled/utils": "0.5.1",
"@emotion/is-prop-valid": "^0.8.6",
"benchmark": "^2.1.4",
"resolve": "^1.17.0"
},
"devDependencies": {
"@types/babel__core": "^7.1.7",
"@types/benchmark": "^1.0.33",
"@types/resolve": "^1.17.1"
"@types/resolve": "^1.17.1",
"benchmark": "^2.1.4"
}
}
22 changes: 13 additions & 9 deletions packages/babel-plugin/src/utils/ast-builders.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,16 @@ const hoistSheet = (sheet: string, meta: Metadata): t.Identifier => {
}

const sheetIdentifier = meta.parentPath.scope.generateUidIdentifier('');
const parent = meta.parentPath.findParent((path) => path.isProgram()).get('body') as NodePath[];
const path = parent.filter((path) => !path.isImportDeclaration())[0];

path.insertBefore(
t.variableDeclaration('const', [t.variableDeclarator(sheetIdentifier, t.stringLiteral(sheet))])
);
const parent = meta.parentPath.findParent((path) => path.isProgram());
const parentBody = parent && (parent.get('body') as NodePath[]);
Copy link
Contributor

Choose a reason for hiding this comment

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

So all of these started breaking just after patch/minor bump ups? 😨

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah

const path = parentBody && parentBody.filter((path) => !path.isImportDeclaration())[0];

path &&
path.insertBefore(
t.variableDeclaration('const', [
t.variableDeclarator(sheetIdentifier, t.stringLiteral(sheet)),
])
);

meta.state.sheets[sheet] = sheetIdentifier;

Expand Down Expand Up @@ -161,12 +165,12 @@ const handleMemberExpressionInStyledInterpolation = (path: NodePath<t.MemberExpr
const propsToDestructure: string[] = [];

if (t.isIdentifier(memberExpressionKey)) {
const traversedUpFunctionPath: NodePath<t.Node> | undefined = path.find((parentPath) =>
const traversedUpFunctionPath: NodePath<t.Node> | null = path.find((parentPath) =>
parentPath.isFunction()
);
const memberExpressionKeyName = memberExpressionKey.name;

const isMemberExpressionNameTheSameAsFunctionFirstParam: boolean =
const isMemberExpressionNameTheSameAsFunctionFirstParam: boolean | null =
traversedUpFunctionPath &&
t.isFunction(traversedUpFunctionPath.node) &&
t.isIdentifier(traversedUpFunctionPath.node.params[0]) &&
Expand Down Expand Up @@ -219,7 +223,7 @@ const handleDestructuringInStyledInterpolation = (path: NodePath<t.Identifier>)
// If we don't skip, `=> load` will not be modified because we have modified `: load` earlier and
// second identifier is nowhere to be found inside function params.
if (path.parentPath && !t.isObjectProperty(path.parentPath.node)) {
const traversedUpFunctionPath: NodePath<t.Node> | undefined = path.find((parentPath) =>
const traversedUpFunctionPath: NodePath<t.Node> | null = path.find((parentPath) =>
parentPath.isFunction()
);

Expand Down
8 changes: 6 additions & 2 deletions packages/babel-plugin/src/utils/ast.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,9 @@ export const resolveIdentifierComingFromDestructuring = ({
let resolvedDestructuringIdentifier: t.ObjectProperty | undefined;

if (t.isObjectPattern(node)) {
return node.properties.find((property) => {
const pattern = node as t.ObjectPattern;

return pattern.properties.find((property) => {
if (t.isObjectProperty(property)) {
if (resolveFor === 'key') {
return t.isIdentifier(property.key) && property.key.name === name;
Expand All @@ -363,9 +365,11 @@ export const resolveIdentifierComingFromDestructuring = ({
return false;
}) as t.ObjectProperty | undefined;
} else if (t.isVariableDeclarator(node)) {
const declarator = node as t.VariableDeclarator;

resolvedDestructuringIdentifier = resolveIdentifierComingFromDestructuring({
name,
node: node.id as t.Expression,
node: declarator.id as t.Expression,
resolveFor,
});
}
Expand Down
2 changes: 1 addition & 1 deletion packages/react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
"csstype": "^2.0.0"
},
"peerDependencies": {
"react": "^16.12.0"
"react": ">= 16.12.0"
},
"devDependencies": {
"@testing-library/react": "^10.0.4",
Expand Down
134 changes: 80 additions & 54 deletions packages/react/src/codemods/codemods-helpers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@ import {
JSXIdentifier,
TSTypeParameter,
Node,
ImportNamespaceSpecifier,
Collection,
} from 'jscodeshift';
import { Collection } from 'jscodeshift/src/Collection';

import { COMPILED_IMPORT_PATH, REACT_IMPORT_PATH, REACT_IMPORT_NAME } from './constants';

type Identifiers = Array<Identifier | JSXIdentifier | TSTypeParameter>;

export const getImportDeclarationCollection = ({
j,
collection,
Expand All @@ -21,11 +23,14 @@ export const getImportDeclarationCollection = ({
j: JSCodeshift;
collection: Collection<any>;
importPath: string;
}) =>
collection
}): Collection<ImportDeclaration> => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Love explicit typings over implicit one. TS compiler will be fast if we specify return types.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we could add an eslint rule for that too.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe later. Should we add card for this? Because it might require changes too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added #422

const found: Collection<ImportDeclaration> = collection
.find(j.ImportDeclaration)
.filter((importDeclarationPath) => importDeclarationPath.node.source.value === importPath);

return found;
};

export const hasImportDeclaration = ({
j,
collection,
Expand All @@ -34,28 +39,39 @@ export const hasImportDeclaration = ({
j: JSCodeshift;
collection: Collection<any>;
importPath: string;
}) =>
getImportDeclarationCollection({
}): boolean => {
const result: Collection<ImportDeclaration> = getImportDeclarationCollection({
j,
collection,
importPath,
}).length > 0;
});

return result.length > 0;
};

export const getImportDefaultSpecifierName = (
importDefaultSpecifierCollection: Collection<ImportDefaultSpecifier>
) => importDefaultSpecifierCollection.nodes()[0]!.local!.name;
): string => {
const name = importDefaultSpecifierCollection.nodes()[0]!.local!.name;
if (!name) {
throw new Error('Name should exist.');
}

return name;
};

export const getImportSpecifierName = (importSpecifierCollection: Collection<ImportSpecifier>) =>
importSpecifierCollection.nodes()[0]!.local!.name;
export const getImportSpecifierName = (
importSpecifierCollection: Collection<ImportSpecifier>
): string | undefined => importSpecifierCollection.nodes()[0]!.local!.name;

export const getAllImportSpecifiers = ({
j,
importDeclarationCollection,
}: {
j: JSCodeshift;
importDeclarationCollection: Collection<ImportDeclaration>;
}) => {
const importSpecifiersImportedNodes: (Identifier | JSXIdentifier | TSTypeParameter)[] = [];
}): Identifiers => {
const importSpecifiersImportedNodes: Identifiers = [];

importDeclarationCollection.find(j.ImportSpecifier).forEach((importSpecifierPath) => {
const node = importSpecifierPath.node.imported;
Expand All @@ -76,8 +92,8 @@ export const findImportSpecifierName = ({
j: JSCodeshift;
importDeclarationCollection: Collection<ImportDeclaration>;
importName: string;
}) => {
const importSpecifierCollection = importDeclarationCollection
}): string | null | undefined => {
const importSpecifierCollection: Collection<ImportSpecifier> = importDeclarationCollection
.find(j.ImportSpecifier)
.filter((importSpecifierPath) => importSpecifierPath.node.imported.name === importName);

Expand All @@ -98,12 +114,14 @@ export const convertDefaultImportToNamedImport = ({
collection: Collection<any>;
importPath: string;
namedImport: string;
}) => {
const importDeclarationCollection = getImportDeclarationCollection({
j,
collection,
importPath,
});
}): void => {
const importDeclarationCollection: Collection<ImportDeclaration> = getImportDeclarationCollection(
{
j,
collection,
importPath,
}
);

importDeclarationCollection.forEach((importDeclarationPath) => {
const importDefaultSpecifierCollection = j(importDeclarationPath).find(
Expand Down Expand Up @@ -154,7 +172,7 @@ export const addCommentBefore = ({
j: JSCodeshift;
collection: Collection<Program>;
message: string;
}) => {
}): void => {
const content = ` TODO(${COMPILED_IMPORT_PATH} codemod): ${clean(message)} `;
collection.forEach((path) => {
path.value.comments = path.value.comments || [];
Expand All @@ -178,7 +196,7 @@ export const addCommentToStartOfFile = ({
j: JSCodeshift;
collection: Collection<Node>;
message: string;
}) => {
}): void => {
addCommentBefore({
j,
collection: collection.find(j.Program),
Expand All @@ -196,13 +214,15 @@ export const addCommentForUnresolvedImportSpecifiers = ({
collection: Collection<Node>;
importPath: string;
allowedImportSpecifierNames: string[];
}) => {
const importDeclarationCollection = getImportDeclarationCollection({
j,
collection,
importPath,
});
const importSpecifiers = getAllImportSpecifiers({
}): void => {
const importDeclarationCollection: Collection<ImportDeclaration> = getImportDeclarationCollection(
{
j,
collection,
importPath,
}
);
const importSpecifiers: Identifiers = getAllImportSpecifiers({
j,
importDeclarationCollection,
});
Expand All @@ -224,8 +244,8 @@ export const addReactIdentifier = ({
}: {
j: JSCodeshift;
collection: Collection<Node>;
}) => {
const hasReactImportDeclaration = hasImportDeclaration({
}): void => {
const hasReactImportDeclaration: boolean = hasImportDeclaration({
j,
collection,
importPath: REACT_IMPORT_PATH,
Expand All @@ -241,19 +261,21 @@ export const addReactIdentifier = ({
);
});
} else {
const importDeclarationCollection = getImportDeclarationCollection({
j,
collection,
importPath: REACT_IMPORT_PATH,
});
const importDeclarationCollection: Collection<ImportDeclaration> = getImportDeclarationCollection(
{
j,
collection,
importPath: REACT_IMPORT_PATH,
}
);

importDeclarationCollection.forEach((importDeclarationPath) => {
const importDefaultSpecifierCollection = j(importDeclarationPath).find(
j.ImportDefaultSpecifier
);
const importNamespaceSpecifierCollection = j(importDeclarationPath).find(
j.ImportNamespaceSpecifier
);
const importDefaultSpecifierCollection: Collection<ImportDefaultSpecifier> = j(
importDeclarationPath
).find(j.ImportDefaultSpecifier);
const importNamespaceSpecifierCollection: Collection<ImportNamespaceSpecifier> = j(
importDeclarationPath
).find(j.ImportNamespaceSpecifier);

const hasNoDefaultReactImportDeclaration = importDefaultSpecifierCollection.length === 0;
const hasNoNamespaceReactImportDeclaration = importNamespaceSpecifierCollection.length === 0;
Expand All @@ -275,12 +297,14 @@ export const replaceImportDeclaration = ({
j: JSCodeshift;
collection: Collection<Node>;
importPath: string;
}) => {
const importDeclarationCollection = getImportDeclarationCollection({
j,
collection,
importPath,
});
}): void => {
const importDeclarationCollection: Collection<ImportDeclaration> = getImportDeclarationCollection(
{
j,
collection,
importPath,
}
);

importDeclarationCollection.forEach((importDeclarationPath) => {
importDeclarationPath.node.source.value = COMPILED_IMPORT_PATH;
Expand All @@ -295,12 +319,14 @@ export const mergeImportSpecifiersAlongWithTheirComments = ({
j: JSCodeshift;
collection: Collection<Node>;
filter?: (name: string | undefined) => boolean;
}) => {
const importDeclarationCollection = getImportDeclarationCollection({
j,
collection,
importPath: COMPILED_IMPORT_PATH,
});
}): void => {
const importDeclarationCollection: Collection<ImportDeclaration> = getImportDeclarationCollection(
{
j,
collection,
importPath: COMPILED_IMPORT_PATH,
}
);

const importSpecifiers: ImportSpecifier[] = [];

Expand Down
Loading