Skip to content

Check for (g/s)etter support, not Object.defineProperty#2087

Merged
sokra merged 1 commit intowebpack:webpack-1from
SimenB:getters-setters
Feb 24, 2016
Merged

Check for (g/s)etter support, not Object.defineProperty#2087
sokra merged 1 commit intowebpack:webpack-1from
SimenB:getters-setters

Conversation

@SimenB
Copy link
Copy Markdown
Contributor

@SimenB SimenB commented Feb 22, 2016

As discussed in #2085

@sokra
Copy link
Copy Markdown
Member

sokra commented Feb 22, 2016

You need to put the function into the module.exports = function() { block. Only these block is emitted into the runtime.

@SimenB
Copy link
Copy Markdown
Contributor Author

SimenB commented Feb 22, 2016

Yeah, I noticed! Running the tests locally now 😄

@SimenB
Copy link
Copy Markdown
Contributor Author

SimenB commented Feb 22, 2016

Done.

BTW, the examples-tests fails for me. Any idea why?

image

@sokra
Copy link
Copy Markdown
Member

sokra commented Feb 22, 2016

npm is weird and installs webpack inside webpack/node_modules. delete it.

@sokra
Copy link
Copy Markdown
Member

sokra commented Feb 22, 2016

Instead clone the webpack repo into a node_modules folder, or npm link webpack into itself (https://github.com/webpack/webpack/blob/master/.travis.yml#L15-L16).

@SimenB
Copy link
Copy Markdown
Contributor Author

SimenB commented Feb 22, 2016

Ah, fancy!

@wkwiatek
Copy link
Copy Markdown

Is it going to be merged into v2 branch as well?

sokra added a commit that referenced this pull request Feb 24, 2016
Check for (g/s)etter support, not Object.defineProperty
@sokra sokra merged commit 729905d into webpack:webpack-1 Feb 24, 2016
@sokra
Copy link
Copy Markdown
Member

sokra commented Feb 24, 2016

Thanks

@sokra
Copy link
Copy Markdown
Member

sokra commented Feb 24, 2016

Is it going to be merged into v2 branch as well?

Nope. v2 depends on Object.defineProperty anyway for the ES6 stuff. v2 doesn't support IE8!

@SimenB SimenB deleted the getters-setters branch February 24, 2016 19:28
@SimenB
Copy link
Copy Markdown
Contributor Author

SimenB commented Mar 2, 2016

@sokra Could you publish this?

@SimenB
Copy link
Copy Markdown
Contributor Author

SimenB commented Mar 18, 2016

@sokra ping 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants