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

Discussion: break up import() that are NOT doing ES import work ? #21910

Closed
headwindz opened this issue Jul 19, 2021 · 4 comments
Closed

Discussion: break up import() that are NOT doing ES import work ? #21910

headwindz opened this issue Jul 19, 2021 · 4 comments

Comments

@headwindz
Copy link
Contributor

Background information

I have been developing a fimga plugin and recently get an exact error as described in figma/plugin-typings#36. The root cause is that the Secure ECMAScript (SES) in figma tries to block any instances of the dynamic import expression in the code being evaluated. However, The SES shim doesn't use a full parser, so they can't tell precisely when the evaluated code contains a real import, but they do employ a regular expression to spot anything that might be a real import expression. The regular expression used is new RegExp('(^|[^.])\\bimport(\\s*(?:\\(|/[/*]))', 'g'), which matches the string import() in Reactlazy.js. Therefore, it's identified as a dynamic import expression, resulting in a false positive.

The discussion I would like to bring up is: whether the import() should be be broken up, as what has been done in React.lazy.js? Though import() doesn't look like a valid Javascript import statement, it does lead to some confusion for some platforms (E.g. figma) which don't have a strict validation. And it seems there is no harm if the import() is also broken up?

@jacty
Copy link

jacty commented Jul 19, 2021

import() is a ES6 feature has been widely supported. https://caniuse.com/?search=import()
And React.lazy() uses it to support Suspense Component, which is a core part of React Concurrent. If you don't use Suspense, I guess, the block of it won't give rise to any issue in your case.

@bvaughn
Copy link
Contributor

bvaughn commented Jul 19, 2021

I think the request here is to break the string "import()" up to avoid a false positive in the external code:

console.error(
'lazy: Expected the result of a dynamic import() call. ' +
'Instead received: %s\n\nYour code should look like: \n ' +
// Break up imports to avoid accidentally parsing them as dependencies.

e.g.

'lazy: Expected the result of a dynamic imp' + 'ort() call. ' + 

I'm not sure what others will think of this request (to modify React source b'c of the way an external project happens to work) but we can leave the request open for folks to comment.

One thing you might do, @n0rush, to increase the visibility for this would be to just open a PR that makes the change you're requesting.

@headwindz
Copy link
Contributor Author

I'm not sure what others will think of this request (to modify React source b'c of the way an external project happens to work)

Yep, I'm not sure either whether this should be done in React so I raised it as a discussion. Also I saw React seems to have done some similar work before.

// Break up imports to avoid accidentally parsing them as dependencies.
'const MyComponent = lazy(() => imp' +
"ort('./MyComponent'))\n\n" +

One thing you might do, @n0rush, to increase the visibility for this would be to just open a PR that makes the change you're requesting.

Sure. PR here :-)

@gaearon
Copy link
Collaborator

gaearon commented Sep 6, 2021

Closed by #21918.

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

No branches or pull requests

4 participants