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

Update dependencies babel 7 #151

Closed
wants to merge 14 commits into from
Closed

Conversation

EECOLOR
Copy link
Member

@EECOLOR EECOLOR commented Oct 10, 2018

Also updated some other dependencies.

@EECOLOR EECOLOR requested a review from klaasman October 10, 2018 21:04
@@ -14,11 +14,12 @@
"npm-run-all": "^4.1.3"
},
"dependencies": {
"@babel/runtime": "^7.0.0",
Copy link
Contributor

@klaasman klaasman Oct 11, 2018

Choose a reason for hiding this comment

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

Shouldn't this dependency be part of the library?
remove this depedency

@klaasman
Copy link
Contributor

as discussed: let's test these changes (both development as deployment) with some real world codebases before merging

@klaasman
Copy link
Contributor

Not sure if it should be part of this PR, but eslint needs some attention:

image

@klaasman
Copy link
Contributor

Take a look at: https://babeljs.io/docs/en/babel-plugin-proposal-decorators#decoratorsbeforeexport

previously we had decorators before exports, but now it defaults to exports before decorators. Here's a discussion about this: tc39/proposal-decorators#69.

I don't have a strong opinion about the issue, but it would be an easier transition if we stick to the old way.

@EECOLOR
Copy link
Member Author

EECOLOR commented Oct 22, 2018

The setting is set to classic right? As far as I can tell classic is 'before export' anyway.

1 similar comment
@EECOLOR
Copy link
Member Author

EECOLOR commented Oct 22, 2018

The setting is set to classic right? As far as I can tell classic is 'before export' anyway.

@klaasman
Copy link
Contributor

There appear to be some changes regarding to css-modules values variables. Previously the @value syntax worked out of the box, but now it seems to be missing.

@klaasman
Copy link
Contributor

Module not found: Error: Can't resolve '@babel/runtime/regenerator'

@EECOLOR
Copy link
Member Author

EECOLOR commented Oct 23, 2018

There appear to be some changes regarding to css-modules values variables. Previously the @value syntax worked out of the box, but now it seems to be missing.

It should still work when I look at the source code: https://github.com/css-modules/css-modules-loader-core/blob/master/src/index.js

@EECOLOR
Copy link
Member Author

EECOLOR commented Oct 23, 2018

I think the problem is caused by having both postcss 6 and 7...

Does the postcss nightmare ever stop?

@EECOLOR
Copy link
Member Author

EECOLOR commented Oct 23, 2018

Ok, so the problem is nasty.

We waited quite some time for cssnano to move to postcss 6. When it did (as version 4 was released) they quite quickly moved to postcss 7. A lot of the 4.0 issues have been fixed later (4.1.x), part of the release that contained these fixes was the move to postcss 7.

Now postcss-modules or actually css-modules-loader-core (and it's dependencies) are still stuck on postcss 6. It seems however that this project is problematic: css-modules/css-modules-loader-core#174

Another one keeping postcss 6 is cssnext: Deprecating cssnext.

I am not seeing a clear way out. When I push away the depression that creeps up and look at the related community from a non-judging / observational standpoint it seems this is not the right technology to be using. I'm sure we are not alone in this struggle, but we should be looking for one of the following:

  • An alternative
  • Copy all related code we need into a repository and clean it up

@EECOLOR
Copy link
Member Author

EECOLOR commented Oct 30, 2018

There is some movement on the css module issues

@klaasman
Copy link
Contributor

klaasman commented Dec 3, 2018

I've been working on a higgins+kaliberjs project while depending on this branch without any issues 👍

@EECOLOR
Copy link
Member Author

EECOLOR commented Dec 8, 2018

It seems there is a solution available for css modules. The webpack css loader plugin solves it by working around the postcss-modules plugin: https://github.com/webpack-contrib/css-loader/blob/master/src/index.js#L77

This should also simplify our css loader plugin.

@EECOLOR
Copy link
Member Author

EECOLOR commented Dec 8, 2018

Support https://github.com/css-modules/postcss-modules-extract-imports is tricky, so we won't add it

@EECOLOR
Copy link
Member Author

EECOLOR commented Feb 6, 2019

The problem of this branch is that it introduces 2 versions of the postcss library. I started working on a solution (going without the unsupported css modules plugin) in the eecolor-css-troubles branch. That work needs to be complete before we can merge it.

A shortterm alternative would be to update non-css related dependencies.

@EECOLOR EECOLOR mentioned this pull request Feb 8, 2019
@EECOLOR
Copy link
Member Author

EECOLOR commented Feb 8, 2019

Closed in favor of #159

@EECOLOR EECOLOR closed this Feb 8, 2019
@EECOLOR EECOLOR deleted the update-dependencies-babel-7 branch February 8, 2019 12:06
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