chore(headless): add case-assist sub package #717

samisayegh · 2021-04-15T19:38:09Z

This PR refactors the rollup file to make it easy to add sub-packages to headless.

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

samisayegh · 2021-04-15T19:40:40Z

packages/headless/contributors/adding-a-sub-package.md

@@ -0,0 +1,36 @@
+# Adding a sub-package


Preview: https://github.com/coveo/ui-kit/blob/KIT-605/packages/headless/contributors/adding-a-sub-package.md

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

github-actions · 2021-04-15T19:56:22Z

Pull Request Report

PR Title

✅ Title follows the conventional commit spec.

Bundle Size

File	Compression	Old (kb)	New (kb)	Change (%)
dist/browser/headless.js	bundled	463.2	463.2	0
	minified	194.9	194.9	0
	gzipped	56.4	56.4	0
dist/browser/headless.esm.js	bundled	449.4	449.4	0
	minified	248.6	248.6	0
	gzipped	62.1	62.1	0
dist/browser/case-assist/headless.js	bundled	0.5	0.5	0
	minified	0.3	0.3	0
	gzipped	0.2	0.2	0
dist/browser/case-assist/headless.esm.js	bundled	0.1	0.1	0
	minified	0	0	0
	gzipped	0.1	0.1	0
../atomic/src/external-builds/headless.esm.js	bundled	449.4	0	-100
	minified	248.6	0	-100
	gzipped	62.1	0	-100

olamothe · 2021-04-15T19:47:36Z

packages/headless/src/case-assist.ts

@@ -0,0 +1,3 @@
+export function hello() {


Remove I guess ;)

Also, I think it would be nicer if we had:

packages/headless/src/case-assist/index.ts

I will empty the file and keep just a comment for git.

For creating a case-assist directory, I feel this divides case-assist functionality into it's own branch of the tree, which I don't think we should do.

Currently it looks like this:

I think we should keep controllers together, features together, and group functionality under those concepts, rather than by sub-package. Some controllers will be used by multiple packages after all.

controllers/ controller1 controller2

If the goal behind the directory is to have the word index in the file name though, we could call it:

case-assist.index.ts

Edit: readding a dummy export because rollup throws an error otherwise.

packages/headless/package.json

packages/headless/pkg/case-assist/package.json

olamothe · 2021-04-15T19:51:13Z

packages/headless/pkg/case-assist/package.json

@@ -0,0 +1,10 @@
+{
+  "name": "case-assist",
+  "version": "1.0.0",


Euhmmm... how will that work exactly with lerna + semantic commit ?

What does that "version" represent ?

I will remove it, it's a result of running npm init to generate this file.

This package.json is just a way to tell IDEs and bundlers where the js and typedefinitions are located for a sub-package. The source of truth for the version published to npm is the one in the headless root package.json

olamothe · 2021-04-15T19:54:31Z

packages/headless/contributors/adding-a-sub-package.md

+
+1. Build the headless project: `npm run build`.
+2. Create a tarball: `npm pack`.
+3. Install the tarball as a dependency of a different project: `npm i <path to the tarball>`.


🤔 ?

Why pack ? npm link does not work in this case ?

The commands are similar, but I think npm pack is more accurate because it creates an output matching what is uploaded npm.

For example, this config in the package.json tells npm what files to include in the tarball. Say someone created a new directory, but did not update the config. I suspect a local symlink would provide access to the new directory, but when installing from npm, the directory would be missing.

Ok.

I just know that personally, I would hate to work with that flow and would not do as described here :D

Seems like a hassle for "maybe it's going to be more accurate".

olamothe · 2021-04-15T19:56:58Z

packages/headless/contributors/adding-a-sub-package.md

+1. Build the headless project: `npm run build`.
+2. Create a tarball: `npm pack`.
+3. Install the tarball as a dependency of a different project: `npm i <path to the tarball>`.
+4. Import an export from your sub-package: `import {...} from '@coveo/headless/pkg/<sub-package>'`


Anyway we can do it without the /pkg/ in there ? it's kind of useless info, since it's always going to be there for all sub packages

ie:

import { buildCaseDeflection } from '@coveo/headless/case-assist

This would be possible, but with one draw-back: the case-assist directory holding the package.json file needs to be at the root level.

My concern was if headless has n packages, then the root level will become cluttered:

dist/ src/ package1/ package2/ package3/ ...

By grouping the sub-packages under a pkg dir, the root is more tidy. Personally, I prefer the shorter path same as you, but I did this as a compromise.

I will check if it is possible to specify a base-path when creating the tarball, but assuming it is not, what do you think between adding the sub-packages to the root vs. grouping them under a dir?

Updates:

Node 12.7 added support for an exports field to allow mapping one path to another.

https://stackoverflow.com/questions/51313753/npm-package-json-base-root-property?noredirect=1#comment89604253_51313753

https://github.com/jkrems/proposal-pkg-exports/#example

However, this does not yet work inside typescript projects. Support is expected as part of TS 4.3 due next month, but it is still not working on the beta.

Support for NodeJS 12.7+ package exports microsoft/TypeScript#33079 (comment)

TypeScript 4.3 Iteration Plan microsoft/TypeScript#42762

Maybe we could start with top-level directories, and refactor once support is available. This would provide our ideal api, and avoid a breaking change.

I will look further tonight and make the decision tomorrow,

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

lbergeron · 2021-04-16T12:49:48Z

packages/headless/rollup.config.js

+  {
+    input: 'src/index.ts',
+    output: [
+      buildUmdOutput('dist/browser', 'CoveoHeadless'),


Do we have any generated documentation that lists the packages and their content?
If yes, where is it?
If not, is that something we plan on providing in the short term?

Just trying to think how customers will find out whether to use CoveoHeadless or CoveoHeadlessCaseAssist.

The documentation does not display the new case-assist module at the moment, but it's something we can address in the short-term. We would add a section on how to consume headless via CDN and can include the name of the injected global.

lbergeron · 2021-04-16T12:59:23Z

packages/quantic/coveo.d.ts

@@ -15,7 +15,7 @@ declare global {
          initialized: boolean;
        }[];
        config: HeadlessConfigurationOptions;
-        engine: HeadlessEngine;
+        engine: HeadlessTypes.HeadlessEngine;


🤔 Maybe I'm just confused, but will that work when passing an engine created through a sub-package? I know that both engines will use the same HeadlessEngine class, but since engines are exposed from two different packages, they will be two different types, no?

At the moment, there is only one engine class so the type will be the same. I'm not sure if separate types are needed just yet. I will investigate further while looking into how to pass different api clients to different engine instances.

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

samisayegh added 6 commits April 9, 2021 18:12

add case-assist sub-module

34e1a86

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

merge master

d9000bf

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

add modules folder

7588462

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

refactor rollup to process multiple modules

d291402

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

place modules in pkg folder

ade2af1

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

add contributors guide

42d06fe

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

samisayegh requested review from btaillon-coveo, olamothe and ThibodeauJF as code owners April 15, 2021 19:38

samisayegh requested review from lbergeron and nathanlb April 15, 2021 19:38

samisayegh commented Apr 15, 2021

View reviewed changes

samisayegh added 2 commits April 15, 2021 15:47

fix typedefinition imports

44ac73c

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

fix dev config

09ef612

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

olamothe approved these changes Apr 15, 2021

View reviewed changes

samisayegh added 3 commits April 15, 2021 16:30

update definition references

cbedcb7

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

fix definitions

710b5b7

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

undo fix

8ddee64

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

lbergeron reviewed Apr 16, 2021

View reviewed changes

samisayegh added 6 commits April 19, 2021 13:48

remove version field

58e576c

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

rename case-assist entry file

4dcf70c

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

readd dummy export

3a321f2

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

merge master

c43fdc9

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

place sub-package at root level; update guide

05c1089

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

update import path

9ad3d86

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

samisayegh merged commit 257001d into master Apr 23, 2021

samisayegh deleted the KIT-605 branch April 23, 2021 13:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(headless): add case-assist sub package #717

chore(headless): add case-assist sub package #717

samisayegh commented Apr 15, 2021

samisayegh Apr 15, 2021

github-actions bot commented Apr 15, 2021 •

edited

Loading

olamothe Apr 15, 2021

olamothe Apr 15, 2021

samisayegh Apr 19, 2021

samisayegh Apr 19, 2021

olamothe Apr 15, 2021

samisayegh Apr 19, 2021

olamothe Apr 15, 2021

samisayegh Apr 19, 2021

olamothe Apr 21, 2021

olamothe Apr 15, 2021

samisayegh Apr 19, 2021 •

edited

Loading

samisayegh Apr 19, 2021 •

edited

Loading

lbergeron Apr 16, 2021

samisayegh Apr 19, 2021

lbergeron Apr 16, 2021

samisayegh Apr 19, 2021 •

edited

Loading

chore(headless): add case-assist sub package #717

chore(headless): add case-assist sub package #717

Conversation

samisayegh commented Apr 15, 2021

Choose a reason for hiding this comment

github-actions bot commented Apr 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samisayegh Apr 19, 2021 • edited Loading

Choose a reason for hiding this comment

samisayegh Apr 19, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samisayegh Apr 19, 2021 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Apr 15, 2021 •

edited

Loading

samisayegh Apr 19, 2021 •

edited

Loading

samisayegh Apr 19, 2021 •

edited

Loading

samisayegh Apr 19, 2021 •

edited

Loading