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

Adding --extractErrors to build throws errors #912

Open
andrewmcgivery opened this issue Oct 27, 2020 · 4 comments
Open

Adding --extractErrors to build throws errors #912

andrewmcgivery opened this issue Oct 27, 2020 · 4 comments
Labels
solution: outdated This is not up-to-date with the current version

Comments

@andrewmcgivery
Copy link

Current Behavior

I'm currently trying to add the --extractErrors flag to builds of Redux Toolkit (https://github.com/reduxjs/redux-toolkit) and without making ANY changes, just adding that flag, it gives some pretty unhelpful errors. I'm not sure what information to provide because I'm a bit lost myself on what is causing this other than "babel issue".

> tsdx build --format cjs,esm,umd --name redux-toolkit --extractErrors && api-extractor run --local

✓ Creating entry file 3.8 secs
Browserslist: caniuse-lite is outdated. Please run next command `npm update`
(at position 1 plugin) SyntaxError: Unexpected token, expected , (47:4)

at undefined:47:4
SyntaxError: Unexpected token, expected , (47:4)
    at Parser.pp$5.raise (E:\Source\redux-toolkit\node_modules\babylon\lib\index.js:4454:13)
    at Parser.pp.unexpected (E:\Source\redux-toolkit\node_modules\babylon\lib\index.js:1761:8)
    at Parser.pp.expect (E:\Source\redux-toolkit\node_modules\babylon\lib\index.js:1749:33)
    at Parser.pp$8.flowParseTypeParameterDeclaration (E:\Source\redux-toolkit\node_modules\babylon\lib\index.js:5322:12)
    at Parser.pp$8.flowParseInterfaceish (E:\Source\redux-toolkit\node_modules\babylon\lib\index.js:5191:32)
    at Parser.pp$8.flowParseInterface (E:\Source\redux-toolkit\node_modules\babylon\lib\index.js:5229:8)
    at Parser.parseExportDeclaration (E:\Source\redux-toolkit\node_modules\babylon\lib\index.js:6036:21)
    at Parser.pp$1.parseExport (E:\Source\redux-toolkit\node_modules\babylon\lib\index.js:2654:29)
    at Parser.parseExport (E:\Source\redux-toolkit\node_modules\babylon\lib\index.js:6000:20)
    at Parser.pp$1.parseStatement (E:\Source\redux-toolkit\node_modules\babylon\lib\index.js:1884:74)

npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! @reduxjs/toolkit@1.4.0 build: `tsdx build --format cjs,esm,umd --name redux-toolkit --extractErrors && api-extractor run --local`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the @reduxjs/toolkit@1.4.0 build script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\Andrew\AppData\Roaming\npm-cache\_logs\2020-10-27T01_44_02_027Z-debug.log

Expected behavior

Shouldn't explode. :) Worth mentioning, ErrorDev, ErrorProd, and codes.json files are created before it craps out. Even if I have invarant being used in the codebase, codes.json is empty (just {}), however.

Suggested solution(s)

I have no idea. I've been trying to dig into this on my side but I've hit a dead end which is why I'm reaching out!

Additional context

Reproducing is really simple... clone Redux Toolkit, add the flag to the build script, and watch it error.

Your environment

System:
    OS: Windows 10 10.0.18363
    CPU: (12) x64 AMD Ryzen 5 2600 Six-Core Processor
    Memory: 8.15 GB / 15.95 GB
  Binaries:
    Node: 12.16.1 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.4 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 6.14.4 - ~\AppData\Roaming\npm\npm.CMD
  Browsers:
    Chrome: 86.0.4240.111
    Edge: Spartan (44.18362.449.0)
    Internet Explorer: 11.0.18362.1
  npmPackages:
    tsdx: ^0.11.0 => 0.11.0
    typescript: ^3.8.2 => 3.8.2
@andrewmcgivery
Copy link
Author

So I noticed the repo isn't using the latest version of tsdx. I upgraded to latest (0.14.1) and it seems to have introduced a completely different problem when I run npm run build.

> tsdx build --format cjs,esm,umd --name redux-toolkit && api-extractor run --local

✓ Creating entry file 1.3 secs
Error: 'import' and 'export' may only appear at the top level

at E:\Source\redux-toolkit\node_modules\symbol-observable\es\index.js:2:0

1: /* global window */
2: import ponyfill from './ponyfill.js';
   ^
3:
4: var root;

@agilgur5
Copy link
Collaborator

agilgur5 commented Oct 27, 2020

So there's a lot to unpack here:

  1. As you later noticed, RTK is on a fairly old version of TSDX:
tsdx: ^0.11.0 => 0.11.0
  1. The error says it's a failure to parse, meaning something was output syntactically incorrect or something.

  2. --extractErrors seems to have very few users because it has a variety of issues and sparse documentation; this is like the second ever issue raised against it.
    It was implemented in Add error code extract and transform #138 by others without any tests. I added some simple tests in Add Integration Tests #627 which failed in multiple different ways, as is written there. I did some further digging into that:

  • warning isn't replaced because this line and this line don't check for warning, only invariant. But even if it were added to those, ErrorProd / ErrorDev don't handle the case of warning; there would have to be quite a few more changes to be made.
  • tiny-invariant's import is never removed, it just becomes a side-effect. The transformErrorMessages Babel plugin doesn't remove it (nor take an identifier for what import to remove since any invariant function can be used)
  • ErrorProd / ErrorDev being in JS and not easily testable is suboptimal and seems problematic
  • My test fixture seems to be a bit buggy (the error basically gets removed as dead code because it uses true) which is why ErrorProd / ErrorDev aren't imported/output properly; they do seem to work when I fix that.
  1. Per deps: update extractErrors plugin to Babel 7 and more #878 this pseudo-fork of React internal scripts is not really maintainable (actual forks tend not to be and this goes several steps further) and was quite difficult to update to Babel 7 (it was previously on a quite outdated version of Babel 6). There are still changes missing from upstream as well. Per there, would much, much, much prefer to depend on a React maintained plugin or something.

It looks like you added a similar, but slightly different plugin in reduxjs/redux#3920, which adds another support for why this should be an entirely independent plugin. If the Redux folks can ask React to split out the plugin, I can chime in there.

@agilgur5 agilgur5 added the solution: outdated This is not up-to-date with the current version label Oct 27, 2020
@agilgur5
Copy link
Collaborator

agilgur5 commented Oct 27, 2020

I upgraded to latest (0.14.1) and it seems to have introduced a completely different problem when I run npm run build.

That's an unrelated error, please keep issues on topic.

I did a search for this error and the first result shows that this is an error upstream in @rollup/plugin-commonjs: rollup/plugins#304. That seems to be fixed in v12.0.0, but that version has an undocumented requirement on Rollup 2, which is a very breaking change slated for v0.15.0.

For the purposes of this specific issue, I'd remove the umd format and see if the original error you reported still occurs, because plugin-commonjs is effectively only used for UMD.
If it does still occur, would need a more minimal reproduction to ascertain the error, but in either case, --extractErrors isn't the highest priority given how it is buggy, unmaintainable, and seemingly not used by very many users

@andrewmcgivery
Copy link
Author

@agilgur5 Apologies for the off topic issue, wasn't sure if it would be helpful or add any context, but yes, appears to be a different issue.

I think I'll wait on v0.15 and see if we can work forward from there on this issue since this is a bit of a rats nest at the moment with multiple issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
solution: outdated This is not up-to-date with the current version
Projects
None yet
Development

No branches or pull requests

2 participants