-
Notifications
You must be signed in to change notification settings - Fork 6
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
fix(lib): Generate a module version of the library to be used as import in a devlopment source #46
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for ods-charts ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
78a8d78
to
5c253eb
Compare
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.
Here are a few first comments for this PR.
@@ -8,7 +8,8 @@ | |||
"files": [ | |||
"./dist/**/*" | |||
], | |||
"main": "./dist/ods-charts.js", |
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 main file here should remain dist/ods-charts.js
because this would be the default packaged version. The module version shouldn't be the default one.
For example, in Bootstrap:
"main": "dist/js/bootstrap.js",
"module": "dist/js/bootstrap.esm.js",
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 test this solution (with the changes in webpack.config.js as well of course), and i got back the angular compilation errors :
./src/app/graph/graph.component.ts:44:21-46 - Error: export 'getThemeManager' (imported as 'ODSCharts') was not found in 'ods-charts' (module has no exports)
./src/app/graph/graph.component.ts:45:14-43 - Error: export 'ODSChartsMode' (imported as 'ODSCharts') was not found in 'ods-charts' (module has no exports)
./src/app/graph/graph.component.ts:46:27-79 - Error: export 'ODSChartsCategoricalColorsSet' (imported as 'ODSCharts') was not found in 'ods-charts' (module has no exports)
./src/app/graph/graph.component.ts:47:24-78 - Error: export 'ODSChartsSequentialColorsSet' (imported as 'ODSCharts') was not found in 'ods-charts' (module has no exports)
./src/app/graph/graph.component.ts:48:19-54 - Error: export 'ODSChartsLineStyle' (imported as 'ODSCharts') was not found in 'ods-charts' (module has no exports)
NB : to update the angular projet before testing, I do
npm uninstall ods-charts
npm i ../..
webpack.config.js
Outdated
...defaultConfig, | ||
output: { | ||
path: path.resolve(__dirname, './dist'), | ||
filename: 'ods-charts-module.js', |
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.
Usually, the naming convention used by libraries is the following:
ods-charts.js
ods-charts.esm.js
ods-charts.umd.js
And for the minified versions:
ods-charts.min.js
ods-charts.esm.min.js
ods-charts.umd.min.js
…rt in a devlopment source
5c253eb
to
d1459eb
Compare
"main": "./dist/ods-charts.js", "module": "./dist/ods-charts.esm.js",
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.
There are maybe some package.lock.json
to regenerate with the latest changes. Could you please merge the main branch inside this one ? I think once done, we might be almost good to go.
Related issues
#18
Description
Angular compilation gave a warning on CommonJS or AMD dependencies
To get code generation optimized, it is recommended that you use ECMAScript modules.
The Pull request contains:
Motivation & Context
Code optimization
Types of change
Test checklist
Please check that the following tests projects are still working:
docs/examples
test/angular-ngx-echarts
test/angular-tour-of-heroes
test/html
test/react
test/vue
test/examples/bar-line-chart
test/examples/single-line-chart