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

bug: Rollup: Parse Error unexpected token #4063

Closed
3 tasks done
mhoritani opened this issue Feb 15, 2023 · 12 comments · Fixed by #5722
Closed
3 tasks done

bug: Rollup: Parse Error unexpected token #4063

mhoritani opened this issue Feb 15, 2023 · 12 comments · Fixed by #5722
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil

Comments

@mhoritani
Copy link

mhoritani commented Feb 15, 2023

Prerequisites

Stencil Version

2.22.0

Current Behavior

HMR breaks build because of an unexpected Token.

image

Expected Behavior

I would like the HMR to work.

System Info

No response

Steps to Reproduce

  • I start our component library with stencil build --dev --watch --serve
  • I edit a component's .tsx file
  • Rebuild occurs and fails with the error unexpected token.

I am having a hard time reproducing this in a clean repo but any information on where I could start investigating would be highly appreciated.

Code Reproduction URL

Additional Information

No response

@ionitron-bot ionitron-bot bot added the triage label Feb 15, 2023
@rwaskiewicz
Copy link
Member

@mhoritani 👋

I have two initial thoughts/questions here:

  • Can you describe your project hierarchy? Does this occur in a project that consumes a separate Stencil project? Or is this a completely self-contained Stencil project?
  • Can you see if this is a reproducible when using Stencil v2.20.0?

Thanks!

@rwaskiewicz rwaskiewicz added the Awaiting Reply This PR or Issue needs a reply from the original reporter. label Feb 15, 2023
@ionitron-bot ionitron-bot bot removed the triage label Feb 15, 2023
@mhoritani
Copy link
Author

Hi @rwaskiewicz, thanks for the reply.

We have a monorepo using npm workspaces and within it a self contained Stencil component library.
I rolled back to 2.20.0 facing the same issue.

@ionitron-bot ionitron-bot bot removed the Awaiting Reply This PR or Issue needs a reply from the original reporter. label Feb 16, 2023
@rwaskiewicz
Copy link
Member

@mhoritani

We have a monorepo using npm workspaces and within it a self contained Stencil component library.
I rolled back to 2.20.0 facing the same issue.

Okay, that helps. It leads me to believe that there's something either in the Stencil or the repo setup that is causing this. When you start Stencil's HMR, are you doing it from the stencil project directly? For example, say you have a repo setup like:

myorg/
├── apps/
├── libs/
    └── stencil-project
├── tools/
├── nx.json
├── package.json
└── tsconfig.base.json

Are you starting HMR from a terminal whose current directory is myorg/libs/stencil-project?

The reason I ask is that in the original screenshot, I noticed ../../node_modules/ in the error message:
Screenshot 2023-02-16 at 8 53 31 AM

If I try to intentionally break Stencil by removing the ':' between SVGTextPathElement and C, I also get an unexpected identifier error, but with a different format.
Screenshot 2023-02-16 at 8 58 03 AM

The important thing I take away is that your error is Rollup-based, whereas mine is not. My approximation/attempt to break Stencil's HMR isn't quite the same as the error you're encountering, which is why I ask more about the project structure and where HMR is being invoked

@rwaskiewicz rwaskiewicz added the Awaiting Reply This PR or Issue needs a reply from the original reporter. label Feb 16, 2023
@mhoritani
Copy link
Author

We have a monorepo using npm workspaces and within it a self contained Stencil component library.
I rolled back to 2.20.0 facing the same issue.

Okay, that helps. It leads me to believe that there's something either in the Stencil or the repo setup that is causing this. When you start Stencil's HMR, are you doing it from the stencil project directly? For example, say you have a repo setup like:

myorg/
├── apps/
├── libs/
    └── stencil-project
├── tools/
├── nx.json
├── package.json
└── tsconfig.base.json

Are you starting HMR from a terminal whose current directory is myorg/libs/stencil-project?

The reason I ask is that in the original screenshot, I noticed ../../node_modules/ in the error message: Screenshot 2023-02-16 at 8 53 31 AM

That's exactly it. Similar structure, stencil build --watch started from the directory where the component lib lives.
The reason the error message says the. node_modules directory is further up is because npm workspaces moves the packages to the top level.

If I try to intentionally break Stencil by removing the ':' between SVGTextPathElement and C, I also get an unexpected identifier error, but with a different format. Screenshot 2023-02-16 at 8 58 03 AM

The important thing I take away is that your error is Rollup-based, whereas mine is not. My approximation/attempt to break Stencil's HMR isn't quite the same as the error you're encountering, which is why I ask more about the project structure and where HMR is being invoked

The most similar reported issue I found so far was #3875.

In this case the culprit was a duplicated import from a barrel file and the file directly - so the error message didn't seem to be closely related unfortunately.

In an effort to confirm it's none of that, I removed all components except for one, rebuilt, checked the components.d.ts file for duplicate imports to no avail.

I want to reiterate that the initial build always works, only upon making changes this issue appears.

Thanks for your efforts!

@ionitron-bot ionitron-bot bot removed the Awaiting Reply This PR or Issue needs a reply from the original reporter. label Feb 16, 2023
@mhoritani
Copy link
Author

Hi @rwaskiewicz!

I have found out what is causing this.

It seems to be related to our functional component unit test. There is no specified way in the Documentation on how to go about this, so we ended up following the approach taken here.

As soon as I remove these tests everything is back to normal.

The reproduction can be found here: https://github.com/mhoritani/stencil-reproduction-4063

Do you have any suggestions on why this has only popped up now or even ideas on how to write these tests in a better way?

@rwaskiewicz
Copy link
Member

@mhoritani

That's interesting, as I wasn't able to reproduce the issue with the reproduction case alone, perhaps something to do with how it's related to NX?

Video following the steps provided:
https://user-images.githubusercontent.com/1930213/220196638-5ce18d0d-85ec-4aaf-b4ce-fd80f8fae777.mov

Do you have any suggestions on why this has only popped up now or even ideas on how to write these tests in a better way?

I couldn't say for sure without additional information, for instance, did you recently update Stencil or any other libraries? If this recently began in your repo, is there a way you could git bisect to find commit that introduced the issue?

@rwaskiewicz rwaskiewicz added the Awaiting Reply This PR or Issue needs a reply from the original reporter. label Feb 20, 2023
@mhoritani
Copy link
Author

mhoritani commented Feb 21, 2023

@rwaskiewicz

We tested this further and found that out of 5 people, only 2 can reproduce it with the provided case as well as within our codebase. Sidenote: We don't use NX, just npm workspaces.

I couldn't say for sure without additional information, for instance, did you recently update Stencil or any other libraries? If this recently began in your repo, is there a way you could git bisect to find commit that introduced the issue?

I tried going back a couple of releases and the issue persisted for me so I don't know if going down that road any further would get us anywhere.

That being said, since it will be rather difficult tracking down variables on different machines causing this, I'm wondering if it would make more sense to focus on the topic of how to properly test functional components? Is there any way Stencil recommends on tackling this? It seems a bit odd to go with our current approach with the @Component decorator within our tests anyway.

Super interested if you have any inputs here!

Thanks a bunch for the support.

@ionitron-bot ionitron-bot bot removed the Awaiting Reply This PR or Issue needs a reply from the original reporter. label Feb 21, 2023
@rwaskiewicz
Copy link
Member

That being said, since it will be rather difficult tracking down variables on different machines causing this, I'm wondering if it would make more sense to focus on the topic of how to properly test functional components? Is there any way Stencil recommends on tackling this? It seems a bit odd to go with our current approach with the @component decorator within our tests anyway.

We can pursue that route - in that case, I can update the name of this issue to better reflect that, get it ingested into our backlog, and move it over to the stencil-site repo (which houses our documentation).

You are correct in that our documentation for testing Functional Components is lacking - beyond a section the describes the differences between class components and functional components we have nearly nothing in with regards to testing.

In the interim, I think the pattern that you're using is nearly there - I would recommend wrapping your functional component in the output of your testing-focused MyFunctionTestComponent and render that as such:

diff --git a/src/components/common/my-functional-component/__tests__/conditional-wrapper.spec.tsx b/src/components/common/my-functional-component/__tests__/conditional-wrapper.spec.tsx
index 9d80e09..c2dda5f 100644
--- a/src/components/common/my-functional-component/__tests__/conditional-wrapper.spec.tsx
+++ b/src/components/common/my-functional-component/__tests__/conditional-wrapper.spec.tsx
@@ -3,7 +3,11 @@ import { newSpecPage, SpecPage } from '@stencil/core/testing';
 import { MyFunctionalComponent } from '../my-functional-component';
 
 @Component({ tag: 'my-functional-component-test' })
-class MyFunctionalTestComponent {}
+class MyFunctionalTestComponent {
+  render() {
+    return <MyFunctionalComponent someProp="🚽"></MyFunctionalComponent>;
+  }
+}
 
 describe('my-functional-component', () => {
   let page: SpecPage;
@@ -11,10 +15,10 @@ describe('my-functional-component', () => {
   beforeEach(async () => {
     page = await newSpecPage({
       components: [MyFunctionalTestComponent],
-      template: () => <MyFunctionalComponent someProp="🚽"></MyFunctionalComponent>,
+      template: () => <my-functional-component-test></my-functional-component-test>,
     });
   });
   it('should render', async () => {
-    expect(page.root).toEqualHtml(`<div>someProp is 🚽</div>`);
+    expect(page.root.firstChild).toEqualHtml(`<div>someProp is 🚽</div>`);
   });
 });

If you have anything specific you're looking for us to document, please feel free to add suggestions via comments on this issue. Thanks!

@rwaskiewicz rwaskiewicz added the Awaiting Reply This PR or Issue needs a reply from the original reporter. label Feb 21, 2023
@rwaskiewicz rwaskiewicz added Bug: Validated This PR or Issue is verified to be a bug within Stencil and removed Awaiting Reply This PR or Issue needs a reply from the original reporter. labels Mar 21, 2023
@goulu
Copy link

goulu commented May 10, 2023

it is definitely a duplicate of my #3875 , which you can close, this one being much better documented.
thanks @mhoritani for the investigation and replication work..

In my situation I had to remove a file from the exports of my custom components library.

// index.ts
export { Components, JSX } from './components';
export * from './utils/some_file';
// export * from './utils/testing'; // causes Rollup: Parse Error in stencil.js see stencil issue #4063

this testing file contains some functions for unit and e2e tests, so it starts with
import * as testing from "@stencil/core/testing";
in case it helps...

@dutscher
Copy link
Contributor

dutscher commented Jun 1, 2023

@goulu we excluded our spec test via tsconfig.json et voila the "unexcepted token" problem is gone.
thanks for that

@christian-bromann
Copy link
Member

This will be fixed with #5722 which enables functional components to be rendered in newSpecPage directly. You can test it making the following changes in the reproduction case linked above:

diff --git a/package.json b/package.json
index 51634df..f6cd181 100644
--- a/package.json
+++ b/package.json
@@ -26,7 +26,7 @@
     "generate": "stencil generate"
   },
   "dependencies": {
-    "@stencil/core": "2.22.2"
+    "@stencil/core": "4.17.2-dev.1714673314.7926a13"
   },
   "devDependencies": {
     "@ionic/prettier-config": "^2.0.0",
diff --git a/src/components/common/my-functional-component/__tests__/conditional-wrapper.spec.tsx b/src/components/common/my-functional-component/__tests__/conditional-wrapper.spec.tsx
index 9d80e09..93eda23 100644
--- a/src/components/common/my-functional-component/__tests__/conditional-wrapper.spec.tsx
+++ b/src/components/common/my-functional-component/__tests__/conditional-wrapper.spec.tsx
@@ -1,16 +1,13 @@
-import { h, Component } from '@stencil/core';
+import { h } from '@stencil/core';
 import { newSpecPage, SpecPage } from '@stencil/core/testing';
 import { MyFunctionalComponent } from '../my-functional-component';

-@Component({ tag: 'my-functional-component-test' })
-class MyFunctionalTestComponent {}
-
 describe('my-functional-component', () => {
   let page: SpecPage;

   beforeEach(async () => {
     page = await newSpecPage({
-      components: [MyFunctionalTestComponent],
+      components: [],
       template: () => <MyFunctionalComponent someProp="🚽"></MyFunctionalComponent>,
     });
   });

@rwaskiewicz
Copy link
Member

The fix for this PR was included in today's v4.18.0 release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants