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.

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

Webpack for LuigiClient #325

Merged
merged 23 commits into from
Jan 18, 2019
Merged

Conversation

parostatkiem-zz
Copy link
Contributor

Description

lerna bootstrap required in /client

Changes proposed in this pull request:

  • bundle LuigiClient with Babel using preset-env
  • add 2 new commands for /client: npm run bundle and npm run bundle-develop
  • remove manual UMD wrapper from the source file (it's now added by Webpack in slightly changed form)
  • source file code is not changed (except removing UMD part). We should introduce class-approach and other things in a separate task. For now, an object is exported. It's up to us how that object is created.

Related issue(s)

Copy link
Contributor

@dariadomagala-sap dariadomagala-sap left a comment

Choose a reason for hiding this comment

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

Right now, we should probably publish only the transpiled /public folder, so we need package.json and readme.md there as well (look how it's solved in core).

Copy link
Contributor

@dariadomagala-sap dariadomagala-sap left a comment

Choose a reason for hiding this comment

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

Please also add the client/public to the lerna.json file.

@@ -2,8 +2,10 @@
"name": "@kyma-project/luigi-client",
Copy link
Contributor

Choose a reason for hiding this comment

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

Please change the name of this package. It won't be the @kyma-project/luigi-client package anymore, the transpiled version will be.

Copy link
Contributor

@bszwarc bszwarc left a comment

Choose a reason for hiding this comment

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

Small comment


If you want to try Luigi out, see the [examples](https://github.com/kyma-project/luigi/tree/master/core/examples).

For documentation on Luigi Core, see [Luigi documentation](https://github.com/kyma-project/luigi/tree/master/docs).
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can change it to:

For more documentation on Luigi framework, see [Luigi documentation](https://github.com/kyma-project/luigi/tree/master/docs).

because it is not only about Luigi Core. 🤔

Copy link
Contributor

@dariadomagala-sap dariadomagala-sap left a comment

Choose a reason for hiding this comment

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

Now I get
GET http://localhost:4200/luigi-client/luigi-client.bundle.js net::ERR_ABORTED 404 (Not Found)

@maxmarkus maxmarkus self-assigned this Jan 10, 2019
Copy link
Contributor

@maxmarkus maxmarkus left a comment

Choose a reason for hiding this comment

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

Docu generation is not working, rest is fine.

"scripts": {
"bundle": "webpack --mode production",
"bundle-develop": "webpack --mode development --watch",
"test": "echo \"Error: no test specified\" && exit 1",
"docu": "npm run docu:validate && npm run docu:generate:section",
"docu:generate:new-file": "documentation build luigi-client.js -f md --markdown-toc=false -o ../docs/luigi-client-api.md",
Copy link
Contributor

Choose a reason for hiding this comment

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

Documentation is not working (path references and some babel errors)

…ebpack-for-Client

# Conflicts:
#	client/luigi-client.js
@parostatkiem-zz parostatkiem-zz added the WIP Work in progress label Jan 14, 2019
@parostatkiem-zz parostatkiem-zz removed the WIP Work in progress label Jan 15, 2019
…ebpack-for-Client

# Conflicts:
#	client/package-lock.json
#	core/package-lock.json
@parostatkiem-zz
Copy link
Contributor Author

parostatkiem-zz commented Jan 15, 2019

The output file is now client/luigi-client.js to keep backwards compatibility. The source file is client/src/luigi-client.js (of course it can be extended to many files via import).

Copy link
Contributor

@dariadomagala-sap dariadomagala-sap left a comment

Choose a reason for hiding this comment

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

Right now we are publishing regular luigi client + bundled luigi client. I don't think that's a good approach.

@@ -0,0 +1 @@
/luigi-client.js
Copy link
Contributor

Choose a reason for hiding this comment

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

But.. we want to publish luigi-client.js. We don't want to publish /src.

@@ -16,6 +16,7 @@
],
"main": "luigi-client.js",
"scripts": {
"install": "npm run bundle",
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this script for?

…ebpack-for-Client

# Conflicts:
#	core/examples/luigi-sample-angular/package-lock.json
#	core/package-lock.json
…ebpack-for-Client

# Conflicts:
#	core/examples/luigi-sample-angular/package-lock.json
#	core/examples/luigi-sample-angular/package.json
#	core/package-lock.json
@@ -205,4 +211,4 @@ This method informs the main application that there are unsaved changes in the c

#### Parameters

- `isDirty` **[boolean](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Boolean)** indicates if there are any unsaved changes on the current page or in the component
- `isDirty` **[boolean](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Boolean)** tells if there are any unsaved changes on the current page or component
Copy link
Contributor

Choose a reason for hiding this comment

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

Revert to the original version in the js file:

`isDirty` **[boolean](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Boolean)** indicates if there are any unsaved changes on the current page or in the component

@parostatkiem-zz parostatkiem-zz merged commit f77f41f into SAP:master Jan 18, 2019
@parostatkiem-zz parostatkiem-zz deleted the webpack-for-Client branch January 18, 2019 09:27
stanleychh pushed a commit to stanleychh/luigi that referenced this pull request Dec 30, 2021
* Bundle LuigiClient with Babel

* Bump luigi-client version

* Update documentation

* remove object.assign polyfill

* Update path in external views

* Update package-locks & gitignore

* Make only bundled version to be published on NPM
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants