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

Remove webpack-directory-name-as-main dependency and corresponding non-standard default module behavior #7511

Closed
wants to merge 2 commits into from

Conversation

epixa
Copy link
Contributor

@epixa epixa commented Jun 20, 2016

There are a bunch of modules that are relying on a custom loader in
webpack to change the expected loading scheme for modules so that a file
with the same name within a module would be treated as a default for
that module. In order to ultimately remove that custom loader, we add
default index.js files that proxy to those existing module defaults.

This is the safest option, as it preserves both the default loading
behavior for the module as well as any direct module traversals to the
real file.

The custom loader is also removed entirely from the project.

CommonJS syntax is used for consistency's sake: some of the modules that
are updated are webpackShims, and they cannot use es6 syntax.

@epixa
Copy link
Contributor Author

epixa commented Jun 20, 2016

@tylersmalley I seem to remember you saying that you wanted to see this gone.

@tylersmalley
Copy link
Contributor

jenkins, test it

@tylersmalley
Copy link
Contributor

The test failure:

ERROR in ./src/ui/public/test_harness/test_harness.js
    Module not found: Error: Cannot resolve 'file' or 'directory' /home/jenkins/workspace/kibana_core_pr/src/ui/public/stack_trace_mapper in /home/jenkins/workspace/kibana_core_pr/src/ui/public/test_harness
     @ ./src/ui/public/test_harness/test_harness.js 24:28-60

@tylersmalley
Copy link
Contributor

Shouldn't we be able to actually remove most of the files, like ace? They are required within the application and not actually a shim.

There are a bunch of modules that are relying on a custom loader in
webpack to change the expected loading scheme for modules so that a file
with the same name *within* a module would be treated as a default for
that module. In order to ultimately remove that custom loader, we add
default index.js files that proxy to those existing module defaults.

This is the safest option, as it preserves both the default loading
behavior for the module as well as any direct module traversals to the
real file.
This custom loader is no longer necessary, and the non-standard behavior
it introduces is not desired.
@epixa epixa self-assigned this Jun 21, 2016
@epixa epixa removed the review label Jun 21, 2016
@spalger
Copy link
Contributor

spalger commented Jun 23, 2016

I fully support removing this behavior, and regret introducing it to being with, but I really don't think we should spread the use of common js. The webpack shims are defined for 3rd part code, which is why they don't use es6, but that doesn't mean we are going to use es5 everywhere..

@epixa
Copy link
Contributor Author

epixa commented Jun 24, 2016

I agree.

@epixa
Copy link
Contributor Author

epixa commented Oct 29, 2016

I'm going to close this PR since this sort of change is done with automation, so I'd need to apply it again to the whole project anyway at this point.

I could never get this build to pass and I'm not quite sure why. The root cause was buried in a lot of indirection pretty deep inside certain abstractions, so I'll need to figure that one out before issuing a new PR.

@epixa epixa closed this Oct 29, 2016
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.

3 participants