Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Extract the search component into
@automattic/search
#46424Extract the search component into
@automattic/search
#46424Changes from 17 commits
226a2b6
0a752e0
89d759d
966e7dc
238f5a1
a0c8d11
ab016ed
12766f4
3d8fb56
5031caf
f9493df
9fa8e9b
6e349fd
6631056
08c3bd4
fe517fd
3271f9d
bc078b9
587db54
ba4a537
790b225
54a1b3b
65cf3b9
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we mention that this based on the existing code in Calypso?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: we haven't released that yet, should it be
1.0.0-alpha
orbeta
or even just1.0.0
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we don't have any tests yet, it seems we can remove
enzyme
from the list of dependencies, no?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @saramarcondes 👋 I noticed that even though the
search
package is written fully in TypeScript, the transpilation for thedist
directories is done by Babel. The TypeScript compiler is used only to emit the type declarations.Was this an intentional decision or is it some copy-paste accident? The compilation could be done 100% by the TypeScript compiler, with a pair of
tsconfig.json
andtsconfig-cjs.json
config files. The@automattic/data-stores
package does that, for example.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an accident of copy and paste from the way the
components
package is configured. Please feel free to open a PR to fix this if you can, I’m not really familiar with how it would work without Babel...Side note: the language picker package probably has the same problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I'll patch both packages in a PR 👍