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

Updated Encore object to class for proxy typehinting #263

Merged

Conversation

florentdestremau
Copy link
Contributor

Hello, I'm trying to address #151 with this PR.

I have an error in my tests:

Public API
    setOutputPath
  Error: Encore.configureRuntimeEnvironment is not a recognized property or method.
  
  - index.js:942 Object.get
    /home/florent/dev/opensource/webpack-encore/index.js:942:27
  - index.js:17 Context.beforeEach
    /home/florent/dev/opensource/webpack-encore/test/index.js:17:13
  - runnable.js:348 callFn
    [webpack-encore]/[mocha]/lib/runnable.js:348:21
  - runnable.js:340 Hook.Runnable.run
    [webpack-encore]/[mocha]/lib/runnable.js:340:7
  - runner.js:309 next
    [webpack-encore]/[mocha]/lib/runner.js:309:10
  - runner.js:339 Immediate.<anonymous>
    [webpack-encore]/[mocha]/lib/runner.js:339:5
  - timers.js:789 runCallback

I'm looking into it but any advice would be welcome so that we all get a proper proxy and autocomplete on ❤️ PHPSTORM ❤️

index.js Outdated

// Proxy the API in order to prevent calls to most of its methods
// if the webpackConfig object hasn't been initialized yet.
const publicApiProxy = new Proxy(publicApi, {
const EncoreProxy = new Proxy(Encore, {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Encore class should be instanciated there:

const EncoreProxy = new Proxy(new Encore(), { /* ... */ });

@Lyrkan
Copy link
Collaborator

Lyrkan commented Feb 12, 2018

Hey @florentdestremau,

Thanks for working on that, only a small thing to fix and all the tests will probably pass :)

@Lyrkan
Copy link
Collaborator

Lyrkan commented Feb 12, 2018

You also forgot to replace some @return {exports} by @return {Encore} for the following methods:

  • autoProvidejQuery
  • enablePostCssLoader
  • enableSassLoader
  • enableLessLoader
  • enableStylusLoader
  • configureBabel
  • enableTypeScriptLoader
  • enableCoffeeScriptLoader
  • enableForkedTypeScriptTypesChecking

@florentdestremau
Copy link
Contributor Author

@Lyrkan
Copy link
Collaborator

Lyrkan commented Feb 12, 2018

@florentdestremau You can leave that one, it represents the config generated by Encore, not the API.

I can't really test if the autocompletion works right now (no PHPStorm) but based on the code it looks good to me :)

@florentdestremau
Copy link
Contributor Author

florentdestremau commented Feb 12, 2018

This doesn't solve the problem for me actually, when I copy / paste my new index.js file into my node_modules. I'm going to try by installing from my fork

Edit:

Confirmed, by using my fork I still get Can't analyse webpack.config.js: coding assistance will ignore module resolution rules in this file

@Lyrkan
Copy link
Collaborator

Lyrkan commented Feb 12, 2018

Did you try closing/reopening PHPStorm and your webpack.config.js file? Maybe there is some sort of JSDoc cache?

@florentdestremau
Copy link
Contributor Author

Yes, tried reinstalling node modules after removing them and no results.
For those wanting to try out, here's the new package.json line to use :

"@symfony/webpack-encore": "https://github.com/florentdestremau/webpack-encore/tarball/fix-phpstorm-unresolved-methods",

@Lyrkan
Copy link
Collaborator

Lyrkan commented Feb 12, 2018

@florentdestremau I'm back on a computer with PHPStorm and it seems to work, maybe that's related to your PhpStorm configuration?

2018-02-12_18-59-19

@florentdestremau
Copy link
Contributor Author

Damn, so this is a new problem on my side now 😅
Well at least this PR seems valid.

@weaverryan
Copy link
Member

Woohoo! Thanks @florentdestremau! Actually, this also didn't work for me at first. Well, it auto-completed only the first 4 methods. The problem was my JavaScript version setting. In PhpStorm, make sure you're using ECMAScript 6 (it defaults to ECMAScript 5.1). This made all the difference for me :)

@weaverryan weaverryan merged commit fb7498d into symfony:master Feb 12, 2018
weaverryan added a commit that referenced this pull request Feb 12, 2018
…orentdestremau)

This PR was squashed before being merged into the master branch (closes #263).

Discussion
----------

Updated Encore object to class for proxy typehinting

Hello, I'm trying to address #151 with this PR.

I have an error in my tests:

```
Public API
    setOutputPath
  Error: Encore.configureRuntimeEnvironment is not a recognized property or method.

  - index.js:942 Object.get
    /home/florent/dev/opensource/webpack-encore/index.js:942:27
  - index.js:17 Context.beforeEach
    /home/florent/dev/opensource/webpack-encore/test/index.js:17:13
  - runnable.js:348 callFn
    [webpack-encore]/[mocha]/lib/runnable.js:348:21
  - runnable.js:340 Hook.Runnable.run
    [webpack-encore]/[mocha]/lib/runnable.js:340:7
  - runner.js:309 next
    [webpack-encore]/[mocha]/lib/runner.js:309:10
  - runner.js:339 Immediate.<anonymous>
    [webpack-encore]/[mocha]/lib/runner.js:339:5
  - timers.js:789 runCallback
```

I'm looking into it but any advice would be welcome so that we all get a proper proxy and autocomplete on :heart: *PHPSTORM* :heart:

Commits
-------

fb7498d Initiated class in proxy, uniformized @returns
79dbee8 Updated Encore object to class for proxy typehinting
@florentdestremau
Copy link
Contributor Author

@weaverryan nice catch ! Indeed it works fine with ES6. Love it ! Should #151 be closed then ?

@Lyrkan
Copy link
Collaborator

Lyrkan commented Feb 13, 2018

Btw it also fixes the issue in VSCode :)

2018-02-13_16-24-41

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants