angular-ui / ui-router

The de-facto solution to flexible routing with nested views in AngularJS
http://ui-router.github.io/
MIT License
13.54k stars 3k forks source link

V1.1.0 - TypeError: Cannot read properties of undefined (reading 'inherit') #3851

Open p3pp8 opened 1 year ago

p3pp8 commented 1 year ago

After updating to version 1.1.0 i'm not able to run my app anymore. I use AngularJS 1.8.3.

TypeError: Cannot read properties of undefined (reading 'inherit')
    at StateParams.$inherit (factory.js:63:9)
    at StateService.href (factory.js:63:9)
    at BaseUrlRule.handler (factory.js:63:9)
    at UrlService.sync (factory.js:63:9)
    at factory.js:63:9
    at factory.js:63:9
    at Array.forEach (<anonymous>)
    at factory.js:63:9
    at Scope.$broadcast (factory.js:63:9)
    at afterLocationChange (factory.js:63:9)
    at factory.js:63:9
    at Scope.$digest (factory.js:63:9)
    at Scope.$apply (factory.js:63:9)
    at bootstrapApply (factory.js:63:9)
    at Object.invoke (factory.js:63:9)
    at doBootstrap (factory.js:63:9)
(

I did not find any changelog and/or migration instructions. Any help is really appreciated.

wawyed commented 1 year ago

Make sure that you manually specify @uirouter/core in your package.json

p3pp8 commented 1 year ago

Hello, thanx for the answer. I did it, i added "@uirouter/core": "^6.1.0" dep into package.json but still have the same error. Do i need to inject it into the angularjs scope?

wawyed commented 1 year ago

Are you using any other packages from uirouter?

p3pp8 commented 1 year ago

Ah sorry, didn't catch! No, no other package, only angularjs and core.

wawyed commented 1 year ago

Which version did you update from?

p3pp8 commented 1 year ago

1.0.30 and it works perfectly!

wawyed commented 1 year ago

I can't see anything that'd make it break. Can you try uirouter/core 6.0.8?

p3pp8 commented 1 year ago

Just tested using core 6.0.8 and that fixed the problem, works like a charm!

p3pp8 commented 1 year ago

6.0.9 breaks as for 6.1.0

wawyed commented 1 year ago

I think I found out what the problem is. Do you have params declarations as strings instead of objects?

p3pp8 commented 1 year ago

Uhmmm, params in state are always declared as objects like this:

$stateProvider
        .state('myStateName', {
            url: '/myPage',
            params: {
                a: null,
                b: null
            }
        });

I've looked into all the project and unfortunately i didn't find any param defined as string, only objects.

wawyed commented 1 year ago

You just showed me the issue :D. The value of the param is null which is not an object :D

p3pp8 commented 1 year ago

Ok, i'm going to cleanup my code! Thank you!!!!

wawyed commented 1 year ago

Well, I think your case is still supported so I'll add a fix for your case!

wawyed commented 1 year ago

@p3pp8 I'm having some issues reproducing the issue in the unit tests. Would you be able to send me a basic state declaration structure that makes this fail? So that I can test my fix.

p3pp8 commented 1 year ago

Hello @wawyed, the example in the post above is extracted from my project (with some renaming) and is a basic state declaration with default params set to null. I think they should've been declared like this:

$stateProvider
        .state('myStateName', {
            url: '/myPage',
            params: {
                a: { value: null },
                b: { value: null }
            }
        });

Thank you for the support!

wawyed commented 1 year ago

If you change it as above the error disappears?

wawyed commented 1 year ago

We have unit tests that test such functionality but they pass

p3pp8 commented 1 year ago

I'm going to try!

p3pp8 commented 1 year ago

Nothing to do, i've changed all params to have { value: null } instead of null but the error is still there...

wawyed commented 1 year ago

Okay... so it's not that.. would you be able to put a breakpoint where it errors and see which param is trying to access?

p3pp8 commented 1 year ago

The project is very complex and loads many angular components with states, i'll try to do my best but it's not as simple as put a breakpoint

christ182 commented 1 year ago

The loop in stateParams.js kept adding the word 'chunk' in parentParamsKeys. (node_modules/@uirouter/core/lib-esm/params/stateParams.js)

I don't know what causes that, but I changed the loop from

for (let j in parentParamsKeys) {
    if (parentParams[parentParamsKeys[j]].inherit == false || inheritList.indexOf(parentParamsKeys[j]) >= 0)
        continue;
    inheritList.push(parentParamsKeys[j]);
    inherited[parentParamsKeys[j]] = this[parentParamsKeys[j]];
}

to

parentParamsKeys.forEach(j => {
    if (parentParams[j].inherit == false || inheritList.indexOf(j) >= 0)
        return;
    inheritList.push(parentParamsKeys[j]);
    inherited[parentParamsKeys[j]] = this[parentParamsKeys[j]];
});

now it works.

p3pp8 commented 1 year ago

I think i have to wait for a new release, but that's not a problem. Thank you guys!

wawyed commented 1 year ago

@christ182 can you inspect parentParamsKeys, check Object.keys(parentParamsKeys) wondering if for some reason there's some enumerable properties added to that array?

christ182 commented 1 year ago

@wawyed Object.keys is working as expected. It's the loop that does not behave properly.

christ182 commented 1 year ago

I found the reason for mine.

Array.prototype.chunk = function(n) {
    if (!this.length) {
        return [];
    }
    return [this.slice(0, n)].concat(
        this.slice(n).chunk(n)
    );
}

for(var i in parents) must include functions as keys.

Although, I have this Array.prototype.chunk code in for some time now. I don't understand how it suddenly broke.

wawyed commented 1 year ago

The reason I said Object.keys() it's because it retrieves the enumerable properties of the array. I wasn't referring to the existing Object.keys.

Regardless I think I have a fix based on your latest comment.

wawyed commented 1 year ago

I do suggest though that you use Object.defineProperty() instead and make chunk non enumerable to avoid further issues.

ckLocal commented 9 months ago

@wawyed Can V1.1.0 be released as soon as possible?

wawyed commented 8 months ago

@ckLocal is this not what you need? https://www.npmjs.com/package/@uirouter/angularjs/v/1.1.0

ckLocal commented 8 months ago

@wawyed Yeah, this is what I want, that's great.

EoinGriffin-AI commented 4 months ago

Hi, I'm running into this same error and I don't see a similarity to @p3pp8's configuration. Note the value of j is 'clear' here. Seems like that's referencing the clear prototype method of the parentParamsKeys array... but why?

image

Here's where my states are defined. Is there an incompatibility here? I'm upgrading from angular-ui-router 0.4.2 to @uirouter/angularjs 1.1.0 (yes, this is a legacy app that I must support)

(function () {
    'use strict';

    var theModule = angular.module('ai.fieldData', [
        'ui.router'
 // Various other imports removed, for reading clarity.
    ]);

    theModule.config(function ($stateProvider, $urlRouterProvider) {
        $stateProvider
            .state('app', {
                url: '',
                templateUrl: 'source/application/application.html',
                controller: 'FieldDataController',
            })
            .state('app.formContainer', {
                abstract: true,
                url: '/locations/:locationId/visits',
                templateUrl: 'source/application/form-container.html',
                controller: 'FormController',
            });

        $urlRouterProvider.otherwise('');
    });
}());
EoinGriffin-AI commented 4 months ago

Indeed that 'clear' is an array prototype method... how could I have possibly misconfigured my states to make this happen?

image

Here's what the loop should look like. Someone forgot to add the hasOwnProperty() check, to omit prototype properties and functions.

                for (var j in parentParamsKeys) {
                    if (!parentParamsKeys.hasOwnProperty(j) || parentParams[parentParamsKeys[j]].inherit == false || inheritList.indexOf(parentParamsKeys[j]) >= 0)
                        continue;
                    inheritList.push(parentParamsKeys[j]);
                    inherited[parentParamsKeys[j]] = this[parentParamsKeys[j]];
                }
wawyed commented 4 months ago

@EoinGriffin-AI You shouldn't make custom methods into prototype enumerable. Can you show me the code that adds those methods?

EoinGriffin-AI commented 4 months ago

@EoinGriffin-AI You shouldn't make custom methods into prototype enumerable. Can you show me the code that adds those methods?

Ah you're right, thanks for pointing out that they're non-standard functions. It's some old custom code added many years ago to our codebase. I can change their definitions to not be enumerable.