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

"Cannot Redefine property" error when importing #3415

Closed
christianacca opened this issue Dec 3, 2016 · 13 comments · Fixed by #3418
Closed

"Cannot Redefine property" error when importing #3415

christianacca opened this issue Dec 3, 2016 · 13 comments · Fixed by #3418

Comments

@christianacca
Copy link

Do you want to request a feature or report a bug?
Bug report

What is the current behavior?

The following import statement:

import { NgTableParams } from 'ng-table';

produces the following error logged to the console:

Cannot redefine property: NgTableParams

If the current behavior is a bug, please provide the steps to reproduce.

Run the demo-app in ng-table where the above import is attempted:

  1. git clone -b webpack-problem https://github.com/esvit/ng-table.git
  2. npm i
  3. npm run setup:ci
  4. cd demo-apps/es6-webpack
  5. npm start

The last command will use webpack-dev-server to launch the demo-app in your default browser.

In chrome this produces the error logged to the console.

What is the expected behavior?

The import of NgTableParams should work

Please mention other relevant information such as the browser version, Node.js version, Operating System and programming language.

Possible related to: #2953

Environment details

  • Chrome: 54.0.2840.99
  • OS: Windows 10
  • Node: 6.2.2
  • npm: 3.10.9
  • webpack: 2.1.0-beta.27
  • typescript@2.0.3 used to compile the library
  • babel used to compile the demo-app that is failing
@TheLarkInn
Copy link
Member

This doesn't appear to be a webpack related error. Is this an issue with typescript seeing a reimport or redefine of this property?

@christianacca
Copy link
Author

christianacca commented Dec 4, 2016

Hi @TheLarkInn,

What make you think it's a typescript issue? I suspect webpack because:

@christianacca
Copy link
Author

christianacca commented Dec 4, 2016

Did a slightly deeper dive into the problem.

I can stop the error from occurring by not re-exporting NgTableParams from another intermediary module

Let me explain by briefly showing the directory tree where NgTableParams module lives:

ng-table (root directory)
  index.ts
  +- src
    +- core
       index.ts
       ngTableParams.ts 

Here are the relevant exports that collectively are causing the problem it would seem:

// src/core/ngTableParams.ts
export class NgTableParams<T> { }
// src/core/index.ts
export * from './ngTableParams';
// index.ts (at the root of the package)
export * from './src/core';

To "fix" things I have to stop re-exporting NgTableParams, thusly:

// src/core/ngTableParams.ts
export class NgTableParams<T> { }
// src/core/index.ts
export { NgTableParams } from './ngTableParams';
// index.ts (at the root of the package)
export { NgTableParams } from './src/core/ngTableParams';

With the above in place the demo-app runs without error. I have committed the above change to another ng-table branch for comparison (here's the exact: commit)


Even more details

Below is a screenshot of the relevant portion of the webpack bundle that fails. The line of code that is throwing is shown highlighted in blue:

image

Below is a screenshot of the same portion of the webpack bundle after applying the workaround of not re-exporting as explained above:

image

@christianacca
Copy link
Author

I have been able to reduce the problem down further.

The problem occurs even with a single export * involved.

Here's are the relevant changes:

// src/core/ngTableParams.ts
export class NgTableParams<T> { }
// src/core/index.ts
export * from './ngTableParams';
// demo-app
// this fails at runtime with error "Cannot redefine property: NgTableParams"
import { NgTableParams } from 'ng-table/src/core';

The commit with the simplified code: esvit/ng-table@a992e82

christianacca pushed a commit to esvit/ng-table that referenced this issue Dec 4, 2016
@sokra
Copy link
Member

sokra commented Dec 4, 2016

Ah I see the issue. I need the check the spec to see if the app or webpack is wrong here...

@sokra
Copy link
Member

sokra commented Dec 4, 2016

Looks like webpack is wrong here.

export * doesn't export a name that is already exported.

@christianacca
Copy link
Author

christianacca commented Dec 4, 2016

OK thanks Tobias for checking the problem out.

christianacca pushed a commit to esvit/ng-table that referenced this issue Dec 4, 2016
sokra added a commit that referenced this issue Dec 4, 2016
sokra added a commit that referenced this issue Dec 4, 2016
@TheLarkInn
Copy link
Member

Thank you for the detailed report. Made this much easier to fix!!

@Jessidhia
Copy link
Member

To add to it: explicit exports take precedence over reexports (e.g. export const X = '' will take precedence over any X coming from an export * from); duplicate reexports of the same symbol (via export * from of different modules) are not allowed (at least by typescript) or must be overridden with an explicit export { theDuplicateName }.

@sokra
Copy link
Member

sokra commented Dec 5, 2016

duplicate reexports of the same symbol (via export * from of different modules) are not allowed

Correction: They are only allowed when they resolve to the same binding.

see http://www.ecma-international.org/ecma-262/6.0/#sec-resolveexport

@christianacca
Copy link
Author

Thanks for the very quick turnaround @sokra.

Do you know when you're intending to publish the next release?

@TheLarkInn
Copy link
Member

We are waiting on a few things to merge before we release. The next release is potentially looking to be an RC.

@christianacca
Copy link
Author

OK thanks for the heads up

christianacca pushed a commit to esvit/ng-table that referenced this issue Dec 6, 2016
christianacca pushed a commit to esvit/ng-table that referenced this issue Dec 6, 2016
nkzawa pushed a commit to nkzawa/webpack that referenced this issue Dec 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants