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

after AOT build and run "Cannot read property 'id' of undefined" at setColumnDefaults #396

Closed
zxshinxz opened this issue Dec 27, 2016 · 8 comments

Comments

@zxshinxz
Copy link

Hi all,

Just wondering if anybody is experiencing error that I am describing here.

[x] bug report => search github for a similar issue or PR before submitting
[ ] feature request
[ ] support request => Please do not submit support request here

Current behavior
screen shot 2016-12-27 at 4 59 43 pm

Expected behavior
sholud be running without error

To reproduce the problem
ngc compile -> rollup bundle -> ran with gulp-webserver

Please tell us about your environment:

Mac 10.11.6, WebStorm, NPM 3.10.8, gulp-webserver

  • Table version: 0.7.x
    4.1.0

  • Angular version: 2.0.x
    2.3.1

  • Browser: [Chrome]

  • Language: [TypeScript 2.0.3]

@zxshinxz
Copy link
Author

I have fixed problem by moving id() into setColumnDefaults() and created new ngx-datatable build. Just wondering if this is problem with my bundling setup

utils/id.ts

export function id() {
  return ('0000' + (Math.random() * Math.pow(36, 4) << 0).toString(36)).slice(-4);
}

util/column-helper.ts

export function setColumnDefaults(columns: any[]) {
  if(!columns) return;

  for(let column of columns) {
    if(!column.$$id) {
-->      column.$$id = ('0000' + (Math.random() * Math.pow(36, 4) << 0).toString(36)).slice(-4);   
  }

    // translate name => prop
    if(!column.prop && column.name) {
      column.prop = camelCase(column.name);
    }

    // format props if no name passed
    if(column.prop && !column.name) {
      column.name = deCamelCase(column.prop);
    }

    if(!column.hasOwnProperty('resizeable')) {
      column.resizeable = true;

@amcdnl
Copy link
Contributor

amcdnl commented Dec 27, 2016

Seems related to your build. Can you make a demo repo?

@sjonany
Copy link

sjonany commented Dec 27, 2016

I also have the same problem. Looks like util = require('../utils') in the line below results to undefined, when I place breakpoint in the browser, leading to the error just described in the title.

var utils_1 = require('../utils');

This is my rollup config, if that's helpful. http://pastebin.com/1ymv3Rw2
Not sure why rollup fails to resolve the require() statement, even with the commonjs plugin.
-- Will reply to thread if I figured it out.

@sjonany
Copy link

sjonany commented Dec 27, 2016

I discovered the problem, and have a workaround for now. But, I don't know what part of rollup causes this.

When I go to the generated rollup bundle file in dist/build.js, I see that the sequences of common js definition and require() are out of order.

var utils_1$1 = index$6;
index$6 = createCommonJs(..);

Since index$6 happens after the utils assignment, utils_1 will still be undefined.
When I manually move utils_1 assignment after index$6 has been defined, the error disappears.

Tried reordering rollup plugins and moved commonJs to the first plugin to run, but still same problem. This looks more like a rollup problem than this module... unless I'm missing a ngx-datatable -specific quirk that makes it not work with rollup.

EDIT:
I think this is a rollup problem with circular dependencies.
rollup/rollup-plugin-commonjs#119
rollup/rollup-plugin-commonjs#105

In this case, column-helper.js is under utils/, but require utils itself.
The tool probably first generates code for column-helper.js (but the "require utils" line will result in undefined), then later generate the utils commonjs module, which exports column-helper.

@amcdnl
Copy link
Contributor

amcdnl commented Dec 27, 2016

Sounds like we should be able to fix. I'll tweak it but will need ur help testing.

@sjonany
Copy link

sjonany commented Dec 27, 2016

Thanks for the quick reply!
Sure, I was thinking the fix would be small changes in js files under release/utils/
If it's easy for you to send me the changed js files, I can patch them locally, rerun rollup and let you know.

@djamison
Copy link

+1 - Looks like this is still an issue in 6.1.2. Happy to help testing.

@pauldaustin
Copy link
Contributor

The issue is that TypeScript does not allow circular dependencies. So within a module you need to reference the other files in that module using relative references rather than importing from the module.

Using the following imports in the src/utils/column-helper.ts works.

import { camelCase, deCamelCase } from './camel-case';
import { id } from './id';

@amcdnl amcdnl closed this as completed in 066a3aa Feb 21, 2017
amcdnl added a commit that referenced this issue Feb 21, 2017
rsparrow pushed a commit to rsparrow/ngx-datatable that referenced this issue Jun 2, 2017
rsparrow pushed a commit to rsparrow/ngx-datatable that referenced this issue Jun 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants