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

Fix webpack build process #934

Merged
merged 2 commits into from
Jul 1, 2019
Merged

Fix webpack build process #934

merged 2 commits into from
Jul 1, 2019

Conversation

auxves
Copy link
Contributor

@auxves auxves commented Jul 1, 2019

Short description of what this resolves:

This PR fixes issues with modules not being bundled.

Changes proposed in this pull request:

  • Stop using webpack-node-externals
  • Ignore the warning from express

How Has This Been Tested?

Tested by building and installing extension VSIX file.

Checklist:

@shanalikhan
Copy link
Owner

  1. Why are we removing webpack-node-externals ?

Webpack allows you to define externals - modules that should not be bundled. When bundling with Webpack for the backend - you usually don't want to bundle its node_modules dependencies. This library creates an externals function that ignores node_modules when bundling in Webpack.

  1. Ignore the warning from express

Is there any alternate way to manually define to ignore the warning.

@auxves
Copy link
Contributor Author

auxves commented Jul 1, 2019

@shanalikhan

Why are we removing webpack-node-externals ?

That package was added to the PR to prevent the warning from express. However, it also stops webpack from bundling necessary dependencies needed by the extension. This is why that error with fs-extra was happening: because it wasn't bundled with the extension. Dependencies should be put in externals only if they already exist on the target system, like built-in node modules (fs, path, etc) or native modules.

Is there any alternate way to manually define to ignore the warning.

Why do we need an alternative way? According to the docs, stats.warningsFilter is the proper way of doing this.

@shanalikhan shanalikhan merged commit 6d6b08e into shanalikhan:v3.4.0 Jul 1, 2019
@shanalikhan
Copy link
Owner

Okay thanks. I will test more scenarios after creating the package and let you know if there is any error.
Else we will release 3.4.0 in coming days.

@shanalikhan
Copy link
Owner

I have tested it, it still doesnt working. same error

@auxves
Copy link
Contributor Author

auxves commented Jul 2, 2019

@shanalikhan After cloning v3.4.0, installing dependencies, building it using vsce package, and installing it on a portable version of VSCode with no data, I got no errors and was presented with the landing page. Can you try doing the same?

@shanalikhan
Copy link
Owner

  1. Using standard vscode in ubuntu
  2. installed dependencies
  3. build package
  4. install on standard vscode
  5. Same error.

Do you want me to try to portable version as well ? I dont think the code difference will make any impact

@auxves
Copy link
Contributor Author

auxves commented Jul 2, 2019

@shanalikhan Have you tried cloning it again before doing those steps?

git clone git@github.com:shanalikhan/code-settings-sync -b v3.4.0

or

git clone https://github.com/shanalikhan/code-settings-sync -b v3.4.0

@shanalikhan
Copy link
Owner

Found it working, need to create package as root user ( although never build using that ).
Thanks

@auxves auxves deleted the fix-fs-extra-not-found branch July 2, 2019 10:51
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.

2 participants