-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
update build configuration to not use dts-cli #3781
Conversation
@zxbodya Go ahead and get this working now that the jest PR is merged |
getSchemaType(test.schema), | ||
`${JSON.stringify(test.schema)} should guess type of ${test.expected}` | ||
).toEqual(test.expected); | ||
test.each(cases.map((c) => [c.expected, c.schema]))( |
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.
replacement for usage of jest-expect-message
to avoid type errors in type definitions it provides
@@ -1,41 +0,0 @@ | |||
$(function () { |
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.
the file is not used anymore, and also has broken link to unpkg
@@ -39,24 +39,6 @@ import Form from '@rjsf/core'; | |||
|
|||
Our latest version requires React 16+. You can also install `react-jsonschema-form` (the 1.x version) which works with React 15+. | |||
|
|||
### As a script served from a CDN |
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.
core.cjs.production.min.js
is to be no longer correct after the changes (new link should be to index.js
and it is also not minified)
However, for both urls - it is a commonjs bundle, which will not work in browser environment because it will try to "require" all the dependencies (react, lodash, etc…)
This can be changes to umd bundle, but that also would not be easy - requiring bunch of script tags with all the dependencies. And so, I believe, it is better to just remove this section entirely to avoid confusion and also to consider umd bundle as deprecated and to remove that in next major version(in case anyone still using it)
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.
If we were to deprecate it, we would want to add documentation now. Probably here? @nickgros ?
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 probably should have always been the UMD bundle. I don't know of a use case for referencing the cjs bundle from a script
Not sure if we should deprecate the UMD bundle, but that can be a separate discussion
packages/chakra-ui/tsconfig.json
Outdated
// todo: | ||
// ../../node_modules/@chakra-ui/menu/dist/declarations/src/use-menu.d.ts:986:61 - error TS2694: Namespace '"node_modules/csstype/index".Property' has no exported member 'ColorAdjust'. |
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.
I'm guessing that you haven't figured out how to pull in the type for the button yet?
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.
replace todo with a comment about skipLibCheck
@@ -39,24 +39,6 @@ import Form from '@rjsf/core'; | |||
|
|||
Our latest version requires React 16+. You can also install `react-jsonschema-form` (the 1.x version) which works with React 15+. | |||
|
|||
### As a script served from a CDN |
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.
If we were to deprecate it, we would want to add documentation now. Probably here? @nickgros ?
packages/fluent-ui/tsconfig.json
Outdated
// todo | ||
// ../../node_modules/@chakra-ui/menu/dist/declarations/src/use-menu.d.ts:986:61 - error TS2694: Namespace '"react-jsonschema-form/node_modules/csstype/index".Property' has no exported member 'ColorAdjust'. | ||
"skipLibCheck": true |
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.
Are these an artifact of a copy from chakra-ui
?
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.
replace todo with a comment about skipLibCheck
?
packages/semantic-ui/tsconfig.json
Outdated
// todo: | ||
// ../../node_modules/semantic-ui-react/dist/commonjs/generic.d.ts(73,73): error TS2344: Type 'TProps' does not satisfy the constraint 'Record<string, any>'. |
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.
More explanation? It looks like semantic ui has a type issue?
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.
replace todo with a comment about skipLibCheck
@zxbodya Since I just released 5.11.1 you'll need to resolve your conflicts. Sorry |
b51483a
to
bbdffec
Compare
## Dev / playground | ||
|
||
- update playground vite config to use sources directly, allowing to reload changes in it without additional build step | ||
- moving from `dts-cli` to use individual dev tools directly, updating package publish config | ||
- tsc for generating type definitions and esm modules | ||
- esbuild for CJS bundle | ||
- rollup for UMD bundle | ||
|
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.
Leave 5.11.2 and move this into 5.12.0. I needed to release the fix for #3805 to unblock my company
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.
done
Updating build configuration to directly use tsc, esbuild and rollup instead of dts-cli.
Also updating publish configuration to include source files, improving debug experience for consumers.
continuation of #3771
Reasons for making this change
Changes
tsc
to generate esm modules and typescript definitions, among other benefits this allows to navigation to sources inside the monorepoesbuild
for creating commonjs and esm bundlerollup
for creating umd bundle from esm bundleChanges in exported artefacts
antd/lib
are not overritten to useantd/es
)Both of this, if needed - are expected to be configured in a bundler on the library consumer side