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 build issues #2001

Merged
merged 1 commit into from
May 31, 2019
Merged

Fix build issues #2001

merged 1 commit into from
May 31, 2019

Conversation

lbogdan
Copy link
Contributor

@lbogdan lbogdan commented May 31, 2019

This fixes the current build issues caused by an apparent bug in babel-eslint@10.0.1 by upgrading to babel-eslint@11.0.0-beta.0 using our babel-eslink fork, from which we removed the patch-eslint-scope reference:

ERROR in /codesandbox/node_modules/babel-eslint/lib/analyze-scope.js
Module not found: Error: Can't resolve './patch-eslint-scope' in '/codesandbox/node_modules/babel-eslint/lib'
 @ /codesandbox/node_modules/babel-eslint/lib/analyze-scope.js 331:4-35
 @ /codesandbox/node_modules/babel-eslint/lib/parse-with-scope.js
 @ /codesandbox/node_modules/babel-eslint/lib/index.js
 @ /codesandbox/node_modules/happypack/loader.js!./src/app/components/CodeEditor/Monaco/workers/linter/index.js

There's also a few small fixes, see the commit message.

@lbogdan lbogdan changed the title Fix build issues [WIP] Fix build issues May 31, 2019
@@ -315,6 +315,7 @@ module.exports = {
resolve: {
mainFields: ['browser', 'module', 'jsnext:main', 'main'],
modules: [
path.resolve(__dirname, '../node_modules'),
Copy link
Member

Choose a reason for hiding this comment

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

Is this one needed as well?

Copy link
Contributor Author

@lbogdan lbogdan May 31, 2019

Choose a reason for hiding this comment

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

Yes, otherwise webpack resolves the below node_modules to the hoisted root node_modules, and not app/node_modules, for some (not yet clear to me) reason.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, that's weird, oh well!

@lbogdan lbogdan changed the title [WIP] Fix build issues Fix build issues May 31, 2019
@lbogdan
Copy link
Contributor Author

lbogdan commented May 31, 2019

Should be ready to merge.

…pe removed, and:

- remove unneeded code from webpack config
- fix PR build URL to use https
- fix the a11y linting bug
- extract jest messages to common
@lbogdan lbogdan merged commit a371481 into codesandbox:master May 31, 2019
@lbogdan lbogdan deleted the fix/build branch May 31, 2019 20:37
@MichaelDeBoey
Copy link
Contributor

@lbogdan Shouldn't we make an issue/PR upstream too to fix the bug? 🤔

@lbogdan
Copy link
Contributor Author

lbogdan commented Jun 17, 2019

@MichaelDeBoey babel/babel-eslint#772

@MichaelDeBoey
Copy link
Contributor

Since it's fixed upstream, I've updated the babel-eslint dependency in #2071

batiskafff pushed a commit to batiskafff/codesandbox-client that referenced this pull request Jun 19, 2019
…pe removed, and: (codesandbox#2001)

- remove unneeded code from webpack config
- fix PR build URL to use https
- fix the a11y linting bug
- extract jest messages to common
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.

3 participants