Skip to content

Conversation

@jeffin143
Copy link
Contributor

@jeffin143 jeffin143 commented Nov 15, 2019

describe your changes...
add alias to false (ignore) and alias to array, Handling : #3178

  • Read and sign the CLA. PRs that haven't signed it won't be accepted.
  • Make sure your PR complies with the writer's guide.
  • Review the diff carefully as sometimes this can reveal issues.
  • Remove these instructions from your PR as they are for your eyes only.

@netlify
Copy link

netlify bot commented Nov 15, 2019

Preview is ready

Built with commit 23e7bf3

https://deploy-preview-3358--webpackjsorg-netlify.netlify.com

@jeffin143
Copy link
Contributor Author

friendly ping @montogeek :)

Thanks

Copy link
Member

@EugeneHlushko EugeneHlushko left a comment

Choose a reason for hiding this comment

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

Types must be added after field declaration

@jeffin143
Copy link
Contributor Author

@EugeneHlushko : friendly ping :)

Copy link
Member

@EugeneHlushko EugeneHlushko left a comment

Choose a reason for hiding this comment

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

Hi @jeffin143
Thank you for update!

Also, sorry for late response, although i think this is not ready yet, please see my comments

};
```

`resolve.alias` when set to `false` will ignore a module.
Copy link
Member

Choose a reason for hiding this comment

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

I am not really clear from reading it, what exactly happens, webpack won't bundle it? e.g. import will not result in an error and its value will be undefined? e.g.

import x from './ignored-module';

console.log(x); // undefined

Or no?

Also text can be improved:

Suggested change
`resolve.alias` when set to `false` will ignore a module.
Setting `resolve.alias` to `false` will tell webpack to ignore a module.

### `resolve.alias`

`object`
`object` `[string]` `boolean: false`
Copy link
Member

Choose a reason for hiding this comment

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

resolve.alias always takes object I guess.
The changes you are recommending is fit for resolve.alias[key],
cause the changes are recommending

resolve: {
  alias: "string";
  or;
  alias: false;
}

Which I think is not a valid.
it should be

resolve: {
  alias: {
    _: string; // or false
  }
}

So the description is not describing that. Could you make those changes ?

Copy link
Member

Choose a reason for hiding this comment

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

You are completely right, great spot!

Copy link
Contributor Author

@jeffin143 jeffin143 Apr 29, 2020

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The all confusion was created because of

https://github.com/webpack/webpack/blob/21a80704ed4478fbfb07d1b786738c18b4f194bf/schemas/WebpackOptions.json#L2022

Resolve.alias.alias

The second alias accepts string bool or [string]

@EugeneHlushko
Copy link
Member

Hi @jeffin143
Can you please have a look at this one?

@jeffin143
Copy link
Contributor Author

Hi @jeffin143
Can you please have a look at this one?

sure I will take a look it this week, completely went of the list

@vercel vercel bot temporarily deployed to Preview April 29, 2020 14:37 Inactive
@vercel vercel bot temporarily deployed to Preview April 29, 2020 14:39 Inactive
@vercel
Copy link

vercel bot commented Apr 29, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/webpack-docs/webpack-js-org/5wp05j7bq
✅ Preview: https://webpack-js-org-git-fork-jeffin143-fix-3178.webpack-docs.now.sh

Copy link
Member

@EugeneHlushko EugeneHlushko left a comment

Choose a reason for hiding this comment

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

just two minor things left

W> [`null-loader`](https://github.com/webpack-contrib/null-loader) will be deprecated in `webpack@5`. use `alias: { xyz$: false }` or absolute path `alias: {[path.resolve(__dirname, "....")]: false }`
W> [`null-loader`](https://github.com/webpack-contrib/null-loader) is deprecated in `webpack@5`. use `alias: { xyz$: false }` or absolute path `alias: {[path.resolve(__dirname, "....")]: false }`

W> `[string]` values are supported since webpack 5.0.0.
Copy link
Member

Choose a reason for hiding this comment

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

It's still in beta, lets just say 5 here too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, Let's get this merged, It has been sitting here for almost 6 months now :)

@vercel vercel bot temporarily deployed to Preview April 29, 2020 17:06 Inactive
Co-Authored-By: Eugene Hlushko <jhlushko@gmail.com>
@EugeneHlushko EugeneHlushko merged commit 6fc84dc into webpack:master May 7, 2020
@EugeneHlushko
Copy link
Member

Thanks!

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