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

chore(headless,atomic): add type "module" to atomic, atomic-react and headless #4442

Merged
merged 21 commits into from
Sep 24, 2024
Merged

chore(headless,atomic): add type "module" to atomic, atomic-react and headless #4442

merged 21 commits into from
Sep 24, 2024

Conversation

louis-bompart
Copy link
Collaborator

@louis-bompart louis-bompart commented Sep 19, 2024 •
edited by alexprudhomme
Loading

https://coveord.atlassian.net/browse/KIT-3580

Unit tests are failing. This is adressed here, once all PRs are approved we will merge in here

Publint status

  • atomic : no errors
  • headless: no errors
  • atomic-react: no errors

Are the types wrong status

  • atomic : errors Internal resolution in node16 + cjs type warnings
  • headless: cjs type warnings
  • atomic-react: cjs type warnings

@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 19, 2024 •
edited
Loading

Pull Request Report

PR Title

✅ Title follows the conventional commit spec.

Live demo links

Bundle Size

File Old (kb) New (kb) Change (%)
case-assist 239.2 239.2 0
commerce 338.1 338.1 0
search 407.7 407.7 0
insight 392.7 392.7 0
recommendation 250 250.1 0
ssr 401.6 401.7 0
ssr-commerce 350.3 350.3 0

SSR Progress

Use case SSR (#) CSR (#) Progress (%)
search 39 44 89
recommendation 0 4 0
case-assist 0 6 0
insight 0 27 0
commerce 0 15 0
Detailed logs search : buildInteractiveResult
search : buildInteractiveInstantResult
search : buildInteractiveRecentResult
search : buildInteractiveCitation
search : buildGeneratedAnswer
recommendation : missing SSR support
case-assist : missing SSR support
insight : missing SSR support
commerce : missing SSR support

alexprudhomme
alexprudhomme reviewed Sep 20, 2024
View reviewed changes
Copy link
Contributor

@alexprudhomme alexprudhomme left a comment

Choose a reason for hiding this comment

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

Great, working on a patch that fixes error for atomic/headless/atomic-react on these tools.

publint.dev
arethetypeswrong

packages/atomic/.storybook/main.mts Outdated Show resolved Hide resolved
packages/headless/doc-parser/package.json Show resolved Hide resolved
This was referenced Sep 23, 2024
Merged
Merged
fbeaudoincoveo
fbeaudoincoveo approved these changes Sep 23, 2024
View reviewed changes
packages/atomic-react/package.json Outdated Show resolved Hide resolved
packages/atomic/.storybook/main.mts Show resolved Hide resolved
patches/fix-esm-import-path+1.10.0.patch Show resolved Hide resolved
@louis-bompart louis-bompart added this to the V3 GA milestone Sep 23, 2024
alexprudhomme added a commit that referenced this pull request Sep 24, 2024
@alexprudhomme @louis-bompart
test(headless): fix unit tests by changing to vitest (#4449)
7cacd90 
To be merged into #4442


Unit tests were failing because Jest has a hard time with ESM modules. I
tried to make it work but it ended up being easier to switch to vitest
altogether. The api are really really similar, it should not change how
we write tests.

## Headless-react unit tests are still failing it is addressed here - >
#4452

---------

Co-authored-by: Louis Bompart <[email protected]>
@alexprudhomme @louis-bompart
chore(headless): fix internal resolution errors in node16 (#4445)
408b4ad 
## Unit tests are failing. This is adressed
[here](https://github.com/coveo/ui-kit/pull/4449/files#diff-75f80b97846615f5b074710648b8191f74aa4f00fd1536c45bc344b284ca8e87),
once all PRs are approved we will merge in
#4442

## [Publint](https://publint.dev/) status
- atomic : no errors
- headless: no errors
- atomic-react: no errors

## [Are the types wrong](https://arethetypeswrong.github.io/) status
- atomic : errors [Internal resolution in
node16](https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/InternalResolutionError.md)
(Do we need to address this ? People don't use atomic with typescript
anyway ?) + cjs type warnings
- headless: cjs type warnings
- atomic-react: cjs type warnings

---------

Co-authored-by: Louis Bompart <[email protected]>
@alexprudhomme alexprudhomme requested a review from a team as a code owner September 24, 2024 12:31
@alexprudhomme alexprudhomme enabled auto-merge (squash) September 24, 2024 12:54
@alexprudhomme
Copy link
Contributor

This PR was split in 4 sections

  1. This PR
  2. chore(headless): fix internal resolution errors in node16 #4445
  3. test(headless): fix unit tests by changing to vitest #4449
  4. test(headless-react): from jest to vitest #4452

@alexprudhomme alexprudhomme merged commit 21896c3 into master Sep 24, 2024
117 checks passed
@alexprudhomme alexprudhomme deleted the type=module branch September 24, 2024 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexprudhomme alexprudhomme alexprudhomme approved these changes

@pathiery pathiery pathiery approved these changes

@fbeaudoincoveo fbeaudoincoveo fbeaudoincoveo approved these changes

@olamothe olamothe Awaiting requested review from olamothe olamothe is a code owner automatically assigned from coveo/search

@y-lakhdar y-lakhdar Awaiting requested review from y-lakhdar y-lakhdar is a code owner automatically assigned from coveo/search

Assignees

@alexprudhomme alexprudhomme

Labels
None yet
Projects
None yet
Milestone
V3 GA
Development

Successfully merging this pull request may close these issues.
4 participants
@louis-bompart @alexprudhomme @fbeaudoincoveo @pathiery