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

"Assignment to itself has no effect" #666

Closed
arcanis opened this issue Jan 12, 2021 · 4 comments
Closed

"Assignment to itself has no effect" #666

arcanis opened this issue Jan 12, 2021 · 4 comments

Comments

@arcanis
Copy link

arcanis commented Jan 12, 2021

My understanding is that third-party packages shouldn't emit warning due to weird syntax (#395 (comment)), but it doesn't seem to be the case every time. I have the following construct coming from got (caused by this line):

url = url;

Which generates the following warning:

> [...]/got/dist/source/core/utils/url-to-options.js: warning: Assignment of "url" to itself has no effect
    6 │     url = url;
      ╵     ~~~~~~~~~
@arcanis
Copy link
Author

arcanis commented Jan 12, 2021

Similarly, I have the following warning:

 > [...]/keyv/src/index.js: warning: This call to "require" will not be bundled because the argument is not a string literal (surround with a try/catch to silence this warning)
    18 │     return new (require(adapters[adapter]))(opts);
       ╵                 ~~~~~~~

While it could be a valid warning, in my case it's not, this code will never be triggered (it's caused by got depending on cacheable-request, itself on keyv, with keyv having optional compatibility with databases 🤷‍♀️). It would be nice to have a way to hide this warning (perhaps in relation to #665).

Edit: The suggestion from #665 is likely good enough for this particular problem - the url = url is probably something you still want to cover though, since this pattern doesn't sound too farfetched to see in the wild and sounds something that linters would be more apt to detect than bundlers.

@evanw
Copy link
Owner

evanw commented Jan 12, 2021

> [...]/got/dist/source/core/utils/url-to-options.js: warning: Assignment of "url" to itself has no effect

I'd like to know what this error message originally looked like before you replaced part of it with [...]. I assume the path didn't originally look like that. Is this file in a node_modules folder?

 > [...]/keyv/src/index.js: warning: This call to "require" will not be bundled because the argument is not a string literal (surround with a try/catch to silence this warning)

This warning exists to tell you about code that isn't being bundled. The alternative of ignoring this code would mean that the generated bundle is likely to fail at run-time since some of the code is missing. It felt like a good idea to have this warning because other bundlers may actually bundle code like this successfully due to dynamic file system emulation, so people may actually expect it to work. And an extra warning is mostly harmless but a silently failing build can be very annoying and hard to debug.

@arcanis
Copy link
Author

arcanis commented Jan 12, 2021

I'd like to know what this error message originally looked like before you replaced part of it with [...]. I assume the path didn't originally look like that. Is this file in a node_modules folder?

Yep. The full path was something like:

pnp:/home/arcanis/app/.yarn/cache/got-somehash/node_modules/got/dist/source/[...]

And an extra warning is mostly harmless but a silently failing build can be very annoying and hard to debug.

Fair enough 👍

@evanw
Copy link
Owner

evanw commented Jan 13, 2021

Ah that makes sense. Looks like these warnings are only ignored for files inside node_modules if the path does not originate from a plugin. This is because this feature was added before plugins were added. Will fix.

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 a pull request may close this issue.

2 participants