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

add .json to require('./package') #349

Closed
wants to merge 1 commit into from
Closed

add .json to require('./package') #349

wants to merge 1 commit into from

Conversation

janpio
Copy link

@janpio janpio commented Jul 26, 2017

This fixes issues for people using got with webpack, which by default has problems with the missing extension.

I know that extensions are optional, but this simple change shouldn't cause any problems but would resolve the problem of all wbepack users trying to use got (or libraries using got).

I agree that normally libraries shouldn't have to adapt to the ecosystem, but in this case the code becomes even more readable and clearer (I at least didn't know that you could just leave off the file extension.)

Why not just use the workaround? Well, it seems to not work in all cases - and I found one of them.

Fixes issues for people using `got` with webpack. which by default has problems with the missing extension.
@kevva
Copy link
Contributor

kevva commented Jul 27, 2017

There are plenty of closed issues about this already https://github.com/sindresorhus/got/search?q=package+json+webpack&type=Issues&utf8=%E2%9C%93. If the suggested solution doesn't work, I'd say it's something wrong with your configuration, or in ionic-app-scripts. This seems like it could cause it https://github.com/ionic-team/ionic-app-scripts/blob/master/config/optimization.config.js#L13-L15.

@kevva kevva closed this Jul 27, 2017
@janpio
Copy link
Author

janpio commented Jul 28, 2017

Fair enough.

But let me ask, what is so important with using the shortcut version without the filename? Is there a advantage?

@janpio
Copy link
Author

janpio commented Jul 28, 2017

Thanks for the pointer to the optimization.config.js - I wasn't aware of that and now created a PR to fix it.

@kevva
Copy link
Contributor

kevva commented Jul 28, 2017

But let me ask, what is so important with using the shortcut version without the filename? Is there a advantage?

There's no real reason to include the extension because of Webpack not being compatible with Node.js.

Repository owner locked and limited conversation to collaborators Oct 26, 2017
Repository owner deleted a comment from janpio Oct 26, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants