Skip to content
This repository has been archived by the owner on Aug 28, 2019. It is now read-only.

p3-preview is unbuildable. Vaadin components on npm are unusable in React apps. #12

Closed
Peppe opened this issue Jun 14, 2018 · 10 comments
Closed
Assignees

Comments

@Peppe
Copy link
Contributor

Peppe commented Jun 14, 2018

How to verify

  1. Take a checkout
  2. Switch to p3-preview branch
  3. run yarn build
  4. Output:
yarn run v1.7.0
$ react-scripts build
Creating an optimized production build...
Failed to compile.

Failed to minify the code from this file:

 	./node_modules/@polymer/polymer/lib/utils/mixin.js:13

Read more here: http://bit.ly/2tRViJ9

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Reasons behind the failure

This is because parts of polymer (and maybe Vaadin) is not precompiled to ES5 before they are released under npm and react-scripts 1.0 does not want to compile modules to minify them, due to the risk of breaking something.

This is a problem with all react projects, not just our starter. Even if you create a react project from scratch and just add say to it, then it will fail in this manner.

Updating to react-scripts 2.0

They have made a change in policy in react-scripts 2.0 and now they build them to minify. With 2.0, yarn build passes. However at runtime when testing the built version, the web components won't work and you have a huge amount of exceptions in the console.

Steps to test with react-scripts 2.0

I will create a new app from scratch with react-scripts 2.0 and add vaadin-date-picker from npm to it. Nothing else.

  1. npx create-react-app@next --scripts-version=2.0.0-next.66cc7a90 react-scripts-2-test
  2. cd react-scripts-2-test
  3. yarn add @vaadin/vaadin-date-picker
  4. Edit src/App.js and add import '@vaadin/vaadin-date-picker' and <vaadin-date-picker></vaadin-date-picker>
  5. yarn start. The component works! (the theme is very off and the component is barely visible, but that is another issue)
  6. yarn build. Passes without issues.
  7. serve -s build App loads up, everything except the date picker is there. There is a barrage of errors in the console, where of 30 first ones are this:
Uncaught DOMException: custom element constructors must call super() first and must not return a different object
    at Object.<anonymous> (http://localhost:5000/static/js/main.e9a02274.js:1:9728)

Additionally there is these two.

Uncaught (in promise) TypeError: Illegal invocation
    at t.value (custom-style.js:81)

Uncaught TypeError: Illegal invocation
    at t.value (custom-style.js:81)

Summary

I have no way to deploy my app anywhere. I can't put it up anywhere without the build so I can only show my app on my personal computers. You can't do anything other that development with React and Vaadin Components on npm, so the combo provides zero business or other kind of value.

@web-padawan
Copy link
Member

Uncaught (in promise) TypeError: Illegal invocation
at t.value (custom-style.js:81)

See Polymer/polymer#5256

Uncaught DOMException: custom element constructors must call super() first and must not return a different object

I suspect this is related to Polymer/tools#398
There is a proper Babel setup in https://github.com/web-padawan/polymer3-webpack-starter
Most important thing is to use @babel/plugin-transform-classes not later than v7.0.0-beta.35
Not sure about the version used by react-scripts.

@Peppe
Copy link
Contributor Author

Peppe commented Jun 14, 2018

react scripts next branch uses plugin-transform-classes v7.0.0-beta.44. I'll test with older versions.

@Peppe
Copy link
Contributor Author

Peppe commented Jun 14, 2018

I locked plugin-transform-classes to beta.35 with npm install --save @babel/[email protected]. I didn't notice a difference in how it worked compared to 44.

@web-padawan
Copy link
Member

Blocked by facebook/create-react-app#932

The stable Babel 7.0.0 no longer has the problem from the above comments, so waiting for CRA to update to it, and then we will have to bump versions here and verify.

@web-padawan
Copy link
Member

Submitted #13 to track the UglifyJS issue which is not something easy to fix from our side.

@amahdy
Copy link
Contributor

amahdy commented Sep 19, 2018

It works with the react-scripts that comes in packages.json. I don't know how much of a difference there is, but for me, I had to run yarn install before yarn build, since I don't have react-scripts linked globally, and it works fine. Here is the output:

yarn run v1.3.2
$ react-scripts build
Creating an optimized production build...
Compiled successfully.

File sizes after gzip:

  38.05 KB  build/static/js/main.82e62e51.js
  121 B     build/static/css/main.0c62578a.css

The project was built assuming it is hosted at the server root.
You can control this with the homepage field in your package.json.
For example, add this to build it for GitHub Pages:

  "homepage" : "http://myname.github.io/myapp",

The build folder is ready to be deployed.
You may serve it with a static server:

  serve -s build

Find out more about deployment here:

  http://bit.ly/2vY88Kr

✨  Done in 7.12s.

@web-padawan
Copy link
Member

Looks like you are not using p3-preview branch. Here is my output:

$ yarn build
yarn run v1.9.4
$ react-scripts build
Creating an optimized production build...
Failed to compile.

Failed to minify the code from this file:

 	./node_modules/@polymer/polymer/lib/utils/mixin.js:13

Read more here: http://bit.ly/2tRViJ9

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
FAIL: 1

@amahdy
Copy link
Contributor

amahdy commented Sep 19, 2018

Tried it one more time and got the same error, tried it one third time and worked. If it's ok with you, I will send you the working project to check it. But clearly it's an issue, especially that it does not work with latest versions of react-scripts. But yarn install works for development experiments.

@web-padawan
Copy link
Member

The roadmap for react-scripts 2.0.0 is available here: facebook/create-react-app#5024

@web-padawan
Copy link
Member

Fixed by #16 on the p3-preview branch, will merge to master once other starters are fixed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants