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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ScopeHoisting] Statically exported members used internally inside the class scope should have exports mapped to module scope when bundled #3172

Closed
Stradivario opened this issue Jun 28, 2019 · 5 comments

Comments

@Stradivario
Copy link
Contributor

馃悰 bug report

馃帥 Configuration (.babelrc, package.json, cli command)

package.json

{
  "scripts": {
    "start": "parcel start ./src/index.html --public-url=/flags",
    "build": "parcel build ./src/index.html --public-url=/flags --experimental-scope-hoisting"
  },
  "dependencies": {
    "@rxdi/core": "^0.6.7",
    "@rxdi/lit-html": "^0.6.7",
    "rxjs": "^6.5.2",
    "@webcomponents/webcomponentsjs": "^1.2.4",
    "@babel/polyfill": "^7.0.0"
  },
  "browserslist": [
    "last 1 chrome versions"
  ]
}

tsconfig.json

{
  "compileOnSave": false,
  "compilerOptions": {
    "baseUrl": ".",
    "sourceMap": true,
    "module": "esnext",
    "declaration": false,
    "moduleResolution": "node",
    "emitDecoratorMetadata": true,
    "experimentalDecorators": true,
    "esModuleInterop": true,
    "target": "es6",
    "typeRoots": [
      "node_modules/@types"
    ],
    "lib": [
      "es2015",
      "es2016",
      "es6",
      "es2017",
      "dom"
    ]
  },
  "include": [
    "./src/**/*"
  ]
}

馃 Expected Behavior

Statically exported members used internally inside the class scope should have exports mapped to module scope when bundled

馃槸 Current Behavior

Statically exported members are not mapped internally inside the scope of the current Class which leading to property of undefined

example:

This peace of code is taken from https://github.com/Polymer/lit-html/blob/master/src/lib/parts.ts#L145

Important part is here:
https://github.com/Polymer/lit-html/blob/master/src/lib/parts.ts#L311

They are creating new Instance of the class NodePart as a factory if this particular part doesn't exist inside the DOM.

The problem here is that when compiled this part of code remains the same without export referencing him to the module scope

Typescript

itemPart = new NodePart(this.options);

Javascript

itemPart = new NodePart(this.options);

Instead it should be

itemPart = new exports.NodePart(this.options);

The case here is that i am getting the following error

Uncaught ReferenceError: NodePart is not defined

When i change the code with exports.NodePart everything is back to normal and i can use my application with Scope hoisting.

Is this normal to behave like this ?

Is it a problem with implementation and how Typescript compile the code without exports.NodePart ?

Is this issue for Typescript team or ParcelJS ?

here is a simple repository reproducing the problem

https://github.com/Stradivario/parcel-scope-hoisting

1.Run npm start (to have server started)
2. Run npm run build command
3. Try to refresh the page after build is success
4. You will get the following error Uncaught ReferenceError: NodePart is not defined
5. Then run command npm run fix and wait for build to finish
6. Refresh the page and you will se that the application is working properly

npm run fix

This command copy file with exports.NodePart property to node_modules folder and then rerun parcel build ./src/index.html --public-url=/flags --experimental-scope-hoisting

Why this thing happen ? Anyone ?

馃實 Your Environment

Software Version(s)
Parcel 1.12.3
Node 10.16.0
npm 6.9.0
Operating System Ubuntu 18.4.2
@Stradivario Stradivario changed the title Statically exported members used internally inside the class scope should have exports mapped to module scope when bundled [ScopeHoisting] Statically exported members used internally inside the class scope should have exports mapped to module scope when bundled Jun 28, 2019
@Stradivario
Copy link
Contributor Author

Pull request with current fix on Polymer lit-html

lit/lit#954

@mischnic
Copy link
Member

mischnic commented Jun 28, 2019

This is a bug in the renaming process, (and I thought we had it fixed: #2986, #2513, #2981):

class NodePart {
    __commitIterable(value) {
        let x = new NodePart();
    }
}

is turned into

class $zoHw$var$NodePart {
    __commitIterable(value) {
        let x = new NodePart(); // NodePart is undefined now
    }
}

@Stradivario
Copy link
Contributor Author

@mischnic i am glad that i face it then!

Also getting the same output trying to build this lib

https://github.com/vaadin/vaadin-router/blob/master/src/router.js#L187

They are using Static methods when initializing constructor properties thus leading to the same error

Regards!

@Stradivario
Copy link
Contributor Author

Any news here guys :) ? @mischnic

@mischnic
Copy link
Member

Please follow #3664 for updates

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

2 participants