fix: remove 'src' dir from being packaged with npm

[bryceosterhaus]

Right now we bundle all src files with our npm packages when we publish. This isn't necessary and also can cause issues with TS projects that use Clay because TS will resolve to the src file before it resolves to the lib's .js + .d.ts files, which should be the primary entry point into the packages.

This should also help speed up build times in DXP and additionally reduce the amount of files we publish.

[bryceosterhaus]

hmm, I think this is actually gonna be blocked by https://liferay.atlassian.net/browse/LPS-188001

I made a pre-release and tried to use with portal and ran into issues when building because the d.ts files generated contain a reference to src, something like import("@clayui/button/src/Button").IProps. I think if we export those props then it should work fine

[bryceosterhaus]

nvm, the reason src was being referenced is because the tsconfig at the root of the repo is telling TS to always reference the src under it's paths when finding @clayui/

[bryceosterhaus]

published a test version @3.105.1-alpha.9. See example publish

[matuzalemsteles]

Oh thanks for that @bryceosterhaus, I'll take a look at the use cases where the reference still contains.

[matuzalemsteles]

I think the problem with this is that we need the storybook to work correctly and also the inference in the IDE for development. I think we can define a new "paths" just for the declaration configuration file so this only changes in the types' build output, what do you think?

[bryceosterhaus]

Yeah that was a concern of mine as well. I figured I would let you investigate it further though since you might have a bit more context.

My main goal was making sure that the published ./lib folder contained no references or dependencies to ./src. Because right now in clay's published packages, it references src which I don't think was originally intended. See line 79 of this file.

Making this change fixed that, but I think we could make this change just in the definitions tsconfig

[matuzalemsteles]

I see @bryceosterhaus, I'll delve deeper into this thanks for the work on this.

[bryceosterhaus]

@matuzalemsteles I worked on it a bit more, I think this seems to work as expected. Also worked for storybook and the ability to update types across packages without having to re-build every time

[bryceosterhaus]

I did a new pre-release(3.106.0-alpha.0) as well and it looks to work in DXP,

[matuzalemsteles]

How strange now that build is adding the package types to each package that depends on it instead of importing by package and not copying the types to each package. I'm investigating this further.

[bryceosterhaus]

@matuzalemsteles let me know if you make any progress on your concerns. I think what I have here should work for our use case and I don't see any issues with the new configs.

Our release of DXP with react 18 is dependent on these changes in clay, because otherwise it causes a types issue. Happy to work on this more if there is an area of concern for you.

[matuzalemsteles]

My concern is that by having types, for example buttons, in different packages, TypeScript may report errors due to having different types for each dependency in different packages.

[bryceosterhaus]

@matuzalemsteles I'm not really sure I understand your concern there.

Each package should now produce .js and .d.ts but no more src files.

For example, looking at my prerelease:

@clayui/autocomplete - the Input component grabs the types from @clayui/form: Input.d.ts

When @clayui/autocomplete is importing the Input type from @clayui/form, it would grab it from here

So I believe all the types should be synced to whatever package is installed in the node_modules. So if for some reason, you use a new version of @clayui/form that no longer has the Input component, then there would be an error, which we would expect and that would happen how it is currently working.

[matuzalemsteles]

@bryceosterhaus I see but for some reason when I get this PR, clean it and build it again this is how the types are generated instead of being imported. That's what I was trying to say, maybe it's the change of "paths".

[matuzalemsteles]

@bryceosterhaus I did some more tests and it seems that this is indeed the problem because we added "paths" to tsconfig.declarations.json.

I think we can solve this just by leaving paths empty and so it uses the types imported from other packages and it also seems that there is no longer a reference to src, at least the case you showed of VerticalNav is no longer importing from src and yes from lib. Can you test with this if it is working well?

"paths": {
-   "@clayui/*": ["packages/clay-*/src"]
}

[bryceosterhaus]

Oh good catch, I think I added that paths to the wrong file, it should be in tsconfig.global.json, which is used for linting TS. The reason we need this is because linting happens before anything is built, and it grabs the packages locally because of the workspace, so when it looks as node_modules for the types, it'll see just a plain package(src, but no lib yet).

Before adding that path to tsconfig.global.json:

After adding that path to tsconfig.global.json:

I updated this PR with the proper fix

[bryceosterhaus]

I updated the path to be identical to the base path.

Since we have configs structured in a flow like this:

tsconfig.json -> tsconfig.declarations.json -> tsconfig.global.json

We really just needed to omit the "paths" in the "declarations" config, so by overriding paths in declarations and then re-setting it in global, we get our intended result.

[matuzalemsteles]

@bryceosterhaus I was investigating this further today and it seems that it is still consuming the packages in the wrong way. I don't know why this is happening, this happens in the case of VerticalNav.

var Trigger: ({ children, className, ...otherProps }: import("packages/clay-button/lib/Button").IProps & React.RefAttributes<HTMLButtonElement>) => JSX.Element;

[bryceosterhaus]

@matuzalemsteles good catch again. I think the reason for this was because then you use "extend" in TS, the path references are generated from the directory that the config is in. So when generating types, the config used was using a baseDir of ../.., referencing back to the root of the repo. This meant that TS would first look for package.json's under the baseDir and use that as the path for importing. This means we would get import('packages/clay-button/... in the generated types.

With my changes, we now get import("@clayui/button/lib/Button") in those generated d.ts

[matuzalemsteles]

LGTM! Thanks @bryceosterhaus