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.

Sign up for GitHub

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

Jump to bottom

Support for ignoring lazy (not weak) async imports #67

Open
jtbandes opened this issue Mar 4, 2021 · 2 comments
Open

Support for ignoring lazy (not weak) async imports #67

jtbandes opened this issue Mar 4, 2021 · 2 comments

Comments

@jtbandes
Copy link

jtbandes commented Mar 4, 2021

The note on allowAsyncCycles gives an example of using it for weak imports:

      // allow import cycles that include an asyncronous import,
      // e.g. via import(/* webpackMode: "weak" */ './file.js')
      allowAsyncCycles: false,

But it would be nice if this also worked for regular async imports that aren't marked with webpackMode: "weak". It sounds like the default mode for async imports is lazy. https://webpack.js.org/api/module-methods/#magic-comments
@jtbandes jtbandes mentioned this issue Mar 4, 2021
Merged
jtbandes added a commit to foxglove/studio that referenced this issue Mar 4, 2021
@jtbandes
Fix 3D panel circular import and Interaction stories (#146)
3ddf0b5 
The main problem here was a circular import that caused a component to be imported as `undefined`. I found a webpack plugin that can help us detect these—turned on warnings for now, but there are too many to fix currently. Also the plugin doesn't understand that our async imports are a valid way of breaking cycles (aackerman/circular-dependency-plugin#67).
One of the stories had a timing issue — I looked deeper for how to address this, but I think some problems stem from hacky use of `ref` as a way of detecting `onMount` in class components... ideally we would convert everything here to hooks, but that is too big a task for this PR, so just adding a band-aid on this story for now.

I also found that the Layout stories pertain to enabling map height rendering for the Cruise internal map, so are irrelevant here (reverting part of cruise-automation/webviz@6eb6fd9).
@jtbandes
Copy link
Author

jtbandes commented Mar 5, 2021

After some further testing I've noticed that this reproduces when using TypeScript with the compiler flag "module": "commonjs".

amacneil pushed a commit to foxglove/studio that referenced this issue Mar 8, 2021
@jtbandes
Fix 3D panel circular import and Interaction stories (#146)
d533f21 
The main problem here was a circular import that caused a component to be imported as `undefined`. I found a webpack plugin that can help us detect these—turned on warnings for now, but there are too many to fix currently. Also the plugin doesn't understand that our async imports are a valid way of breaking cycles (aackerman/circular-dependency-plugin#67).
One of the stories had a timing issue — I looked deeper for how to address this, but I think some problems stem from hacky use of `ref` as a way of detecting `onMount` in class components... ideally we would convert everything here to hooks, but that is too big a task for this PR, so just adding a band-aid on this story for now.

I also found that the Layout stories pertain to enabling map height rendering for the Cruise internal map, so are irrelevant here (reverting part of cruise-automation/webviz@6eb6fd9).
amacneil pushed a commit to foxglove/studio that referenced this issue Mar 8, 2021
@jtbandes
Fix 3D panel circular import and Interaction stories (#146)
440b9a8 
The main problem here was a circular import that caused a component to be imported as `undefined`. I found a webpack plugin that can help us detect these—turned on warnings for now, but there are too many to fix currently. Also the plugin doesn't understand that our async imports are a valid way of breaking cycles (aackerman/circular-dependency-plugin#67).
One of the stories had a timing issue — I looked deeper for how to address this, but I think some problems stem from hacky use of `ref` as a way of detecting `onMount` in class components... ideally we would convert everything here to hooks, but that is too big a task for this PR, so just adding a band-aid on this story for now.

I also found that the Layout stories pertain to enabling map height rendering for the Cruise internal map, so are irrelevant here (reverting part of cruise-automation/webviz@6eb6fd9).
amacneil pushed a commit to foxglove/studio that referenced this issue Mar 8, 2021
@jtbandes @amacneil
Fix 3D panel circular import and Interaction stories (#146)
4bb6b53 
The main problem here was a circular import that caused a component to be imported as `undefined`. I found a webpack plugin that can help us detect these—turned on warnings for now, but there are too many to fix currently. Also the plugin doesn't understand that our async imports are a valid way of breaking cycles (aackerman/circular-dependency-plugin#67).
One of the stories had a timing issue — I looked deeper for how to address this, but I think some problems stem from hacky use of `ref` as a way of detecting `onMount` in class components... ideally we would convert everything here to hooks, but that is too big a task for this PR, so just adding a band-aid on this story for now.

I also found that the Layout stories pertain to enabling map height rendering for the Cruise internal map, so are irrelevant here (reverting part of cruise-automation/webviz@6eb6fd9).
amacneil pushed a commit to foxglove/studio that referenced this issue Mar 8, 2021
@jtbandes
Fix 3D panel circular import and Interaction stories (#146)
490f6ce 
The main problem here was a circular import that caused a component to be imported as `undefined`. I found a webpack plugin that can help us detect these—turned on warnings for now, but there are too many to fix currently. Also the plugin doesn't understand that our async imports are a valid way of breaking cycles (aackerman/circular-dependency-plugin#67).
One of the stories had a timing issue — I looked deeper for how to address this, but I think some problems stem from hacky use of `ref` as a way of detecting `onMount` in class components... ideally we would convert everything here to hooks, but that is too big a task for this PR, so just adding a band-aid on this story for now.

I also found that the Layout stories pertain to enabling map height rendering for the Cruise internal map, so are irrelevant here (reverting part of cruise-automation/webviz@6eb6fd9).
@Smoothsmith Smoothsmith mentioned this issue Jun 10, 2021
Open
@ferm10n
Copy link

ferm10n commented Oct 2, 2023 •
edited

I'd love to fix this, but I haven't any clue how to tell from a Dependency / ModuleDependency that it's imported via import().

Here's what circular-dependency-plugin looks at:

circular-dependency-plugin/index.js

Line 104 in 21777bf

if (this.options.allowAsyncCycles && dependency.weak) { continue }

And the source from webpack:
https://github.com/webpack/webpack/blob/1f13ff9fe587e094df59d660b4611b1bd19aed4c/lib/Dependency.js#L100

It's really frustrating how hard it is to find a reference for webpack APIs like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jtbandes @ferm10n