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

Angular optimization causes problem because of the use of constructor.name #178

Open
DGolverdingen opened this issue May 30, 2018 · 17 comments
Assignees
Labels

Comments

@DGolverdingen
Copy link

Overview of the issue:

When upgrading project from 4.4.1 -> 5.0.0, getSession() fails to return jsdoSession with form authentication when webapplication is hosted on external webserver. Running the web application on local webserver does not result in the same problem.
We have a test webserver running in Firebase hosting.

Motivation for or use case: Not able to use this in test / production environment
JSDO version: 5.0.0
PAS server: 11.6.4

I am really not sure what is going on.
What is changed between 4.4.1 and 5.0.0 for getSession and Form Auth? And why does is only fail when hosted external?

@DGolverdingen
Copy link
Author

After some more investigation it turns out that the change between adding the progress.js file locally in the project (4.4.1) or importing it from npm (@progress/jsdo-core 5.0.0) causes the problem!

It is a Angular 6 project (with Webpack 4) and something gets messed up with an production build with the following settings in angular.json:
image
Turning optimization to false is fixing my problem.

So please deeply investigate what this build option is doing with the imported module..

(This is the second problem I encounter with a Progress module and Angular 6: telerik/kendo-angular#1542)

@edselg
Copy link
Contributor

edselg commented May 30, 2018

Hello @DGolverdingen

Thank you for submitting the issue.

This is an interesting case. We did not get to see this during our testing with FORM based services.

Thank you for the additional info.

Do you see any error messages in the JavaScript Console or in the Network view?

Would it be possible for you to provide code to reproduce the issue?
You can use the test service listed in the following wiki page:

@DGolverdingen
Copy link
Author

Hi Edsel,

No error, not network problems, no console info.
the getSession() function just returns an object with jsdo session missing.

I will create a demo project in Angular 6.

@DGolverdingen
Copy link
Author

DGolverdingen commented May 30, 2018

Github demo repo: https://github.com/DGolverdingen/angular6jsdo

run ng s (non production build) will result in the following webpage showing what getSession() returns:
image

running ng s --prod will result in a production build with the problem:
image

(Make sure you run this with node.js 8 or higher and the latest angular/cli installed globally)

Turning off optimization in angular.json and the production build will be ok.
image

Ignore the false warning in console:
image
This import is not needed in web application but dependency exist because of require statement that isn't hit.

@edselg edselg self-assigned this Jun 1, 2018
@edselg
Copy link
Contributor

edselg commented Jun 2, 2018

Hello @DGolverdingen

Thank you for the reproducible code.
I was able to reproduce the issue using it.

This issue happens because we use constructor.name to set the jsdosession property for the object returning in an ES6 Promise.
Link: https://github.com/progress/JSDO/blob/master/src/progress.util.js#L262

The production build in Angular minifies progress.core.js in a way that the names for the functions (constructors) are removed. This causes code in progress.util.js to fail.

There are ways to minify and keep the function names, however, I do not know if there is a way to do that via the angular.json file.

I will look into ways to void this scenario, perhaps, by setting a property, for example "_name" to the function and using it from progress.util.js instead of "constructor.name".
Example:

    };   // end of JSDOSession
    progress.data.JSDOSession._name = "JSDOSession";

@DGolverdingen
Copy link
Author

Hi Edsel,

There might be ways, but not with the Angular CLI. Losing the CLI just to be able to use this lib is not a desirable option.
https://stackoverflow.com/questions/44160894/angular-cli-how-to-ignore-class-names-from-being-minified

So please investigate other options!

David

@DGolverdingen
Copy link
Author

Hi Edsel,

Any updates regarding this issue?

Best Regards,

David

@edselg edselg assigned joshualan and unassigned edselg Jun 27, 2018
@edselg
Copy link
Contributor

edselg commented Jul 10, 2018

@DGolverdingen

Update: We are working on a change to the JSDO library to avoid using constructor.name.

Thank you and regards.

@joshualan
Copy link
Collaborator

joshualan commented Jul 10, 2018

@DGolverdingen I've added the fix, it's currently in the develop branch. Let me know if you need a lib file or the updated jsdo-core package.

@DGolverdingen
Copy link
Author

Thanks @edselg and @joshualan!

Is it possible to npm install from dev like npm install @progress/jsdo-core-dev
of do I need to do npm install 'https://github.. etc'

@joshualan
Copy link
Collaborator

joshualan commented Jul 11, 2018

I'll see about updating the public npm repo but I've uploaded a tar file for you to install from for your ease that has the changes you're looking for using npm install jsdo-core.tar.gz.

jsdo-core.tar.gz

@DGolverdingen
Copy link
Author

This does not fix the problem. Please reopen the issue. I updated the repo with the package from the develop branch.

Test it for yourself: https://github.com/DGolverdingen/angular6jsdo
See instruction above.

Sorry for my late response, but I was on holiday.

@joshualan @edselg

@DGolverdingen DGolverdingen changed the title When upgrading project from 4.4.1 -> 5.0.0, getSession() fails to return jsdoSession when hosted on external webserver Angular optimization causes problem because of the use of constructor.name Aug 14, 2018
@joshualan joshualan reopened this Aug 20, 2018
@joshualan
Copy link
Collaborator

joshualan commented Aug 21, 2018

@DGolverdingen I just checked this and the issue is that your new repository doesn't pick up the new changes. Revert the app.component.ts to use @progress/jsdo-core and do an npm install from the tarball a few comments above instead of the jsdo/develop branch and it works!

Let me know if it works if you do it this way and I can close this issue.

@edselg
Copy link
Contributor

edselg commented Aug 22, 2018

Internal issue # ADAS-7038.

@DGolverdingen
Copy link
Author

Ok, I can confirm that it works with the tarball. I still don't understand why it's not working with the develop branch.

Now fighting my next problem.. getting error in CI build with tarball in package-lock.json.

I would really appreciate a new jsdo release. What is the planning for 5.0.1 or 5.1.0?

@edselg
Copy link
Contributor

edselg commented Aug 27, 2018

Hello @DGolverdingen

Thank you for confirming that it works with the tarball.

I think that the issue with the develop branch is because the files are not up to date.
Are you building the files from the develop branch or using files directly?

We have plans for a new release with some enhancements and bug fixes.
However, we do not have an estimated date for it.

Thank you and regards.

@DGolverdingen
Copy link
Author

I did an npm install directly from git with develop branch as target.

@edselg edselg added the bug label Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants