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

Upgrading to 2.0.1 results in 'return' outside of function error #78

Closed
n8sabes opened this issue Dec 17, 2019 · 12 comments
Closed

Upgrading to 2.0.1 results in 'return' outside of function error #78

n8sabes opened this issue Dec 17, 2019 · 12 comments

Comments

@n8sabes
Copy link

n8sabes commented Dec 17, 2019

It seems that upgrading to node-deep-equal 2.0.1 results in an error. I'm seeing other posts on this issue within the last few days against other projects, so it's unclear the true origin.

./node_modules/is-map/index.js
SyntaxError .../node_modules/is-map/index.js: 'return' outside of function (12:1)

  10 | 		return false;
  11 | 	};
> 12 | 	return;
     | 	^
  13 | }
  14 |
  15 | var $mapHas = $Map ? Map.prototype.has : null;

DOWNGRADING to deep-equal 1.1.1 resolves the problem in my React (CRA) project.

@ljharb
Copy link
Member

ljharb commented Dec 17, 2019

This isn’t an error; top level returns are perfectly valid in a CJS module. Have you customized your CRA config at all?

@n8sabes
Copy link
Author

n8sabes commented Dec 17, 2019

Hi @ljharb, I recall the project was a clean CRA --typescript base, but the project has evolved into a more mature project over the last month.

I upgrade packages once a week during development cycles with yarn upgrade --latest, which I did tonight, when the error manifested. Upon rolling back packages one at a time, I found rolling back deep-equal to 1.1.1 resolve the build error.

@ljharb
Copy link
Member

ljharb commented Dec 17, 2019

Indeed; v2 is the one that uses is-map/is-set, which have a top-level return.

The issue is that something that's interacting with node-style CJS modules isn't supporting a decade-old feature of those modules. I'd be surprised if it was Webpack.

@ljharb
Copy link
Member

ljharb commented Dec 17, 2019

eg, webpack/webpack#8509 is resolved, webpack/webpack#8510 is merged, released in webpack v4.31+. What version of webpack are you using?

@n8sabes
Copy link
Author

n8sabes commented Dec 17, 2019

@ljharb, I'm not including webpack myself, but executing npm list webpack reveals the following:

└─┬ react-scripts@3.3.0
  └── webpack@4.41.2

@n8sabes
Copy link
Author

n8sabes commented Dec 17, 2019

Here is a simple set of steps to create a clean project that exhibits the anomaly:

yarn create react-app my-app --template typescript
cd my-app
yarn add deep-equal
vi src/index.tsx

# Add lines to index.tsx before render()
    import deepEqual from "deep-equal"; // NEW LINE
    deepEqual({}, {});                  // NEW LINE

yarn build

Output:

Failed to compile.

./node_modules/is-map/index.js
SyntaxError: .../my-app/node_modules/is-map/index.js: 'return' outside of function (12:1)

  10 | 		return false;
  11 | 	};
> 12 | 	return;
     | 	^
  13 | }
  14 |
  15 | var $mapHas = $Map ? Map.prototype.has : null;


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

@mahnunchik
Copy link

The same issue...

@ljharb
Copy link
Member

ljharb commented Dec 17, 2019

Looks like babel needs to have the allowTopLevelReturn option enabled if you're going to be transpiling CJS node_modules.

@n8sabes
Copy link
Author

n8sabes commented Dec 17, 2019

@ljharb, deep-equal is still not working with a vanilla CRA project.

I just recreated a clean my-app with deep-equal as outlined above, but get the same error from is-weakmap:

./node_modules/is-weakmap/index.js
SyntaxError: .../my-app/node_modules/is-weakmap/index.js: 'return' outside of function (12:1)

  10 | 		return false;
  11 | 	};
> 12 | 	return;
     | 	^
  13 | }
  14 |
  15 | var $mapHas = $WeakMap ? $WeakMap.prototype.has : null;

Are you saying you have to somehow modify a CRA install with allowTopLevelReturn to use deep-equal? If so, how do you do that, or am I misunderstanding?

@n8sabes
Copy link
Author

n8sabes commented Dec 17, 2019

Can we re-open this issue until there is a solution to use deep-equal again with CRA generated React apps?

@ljharb
Copy link
Member

ljharb commented Dec 17, 2019

We could have, but you opened up #79 before i had a chance to.

@n8sabes
Copy link
Author

n8sabes commented Dec 17, 2019

Sorry about that -- Thought we lost you already on this issue thread. Thank you for your efforts on this! Sincerely appreciated.

Moving over to #79.

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

No branches or pull requests

3 participants