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

fix: redirect named import #6061

Merged
merged 7 commits into from
Mar 17, 2023
Merged

fix: redirect named import #6061

merged 7 commits into from
Mar 17, 2023

Conversation

ClarkXia
Copy link
Collaborator

@ClarkXia ClarkXia commented Mar 15, 2023

fix: #6059

@codecov-commenter
Copy link

codecov-commenter commented Mar 15, 2023

Codecov Report

❗ No coverage uploaded for pull request base (release/next@aba2554). Click here to learn what that means.
Patch coverage: 94.59% of modified lines in pull request are covered.

Additional details and impacted files
@@               Coverage Diff               @@
##             release/next    #6061   +/-   ##
===============================================
  Coverage                ?   82.06%           
===============================================
  Files                   ?      199           
  Lines                   ?    18256           
  Branches                ?     2379           
===============================================
  Hits                    ?    14982           
  Misses                  ?     3240           
  Partials                ?       34           
Impacted Files Coverage Δ
packages/ice/src/service/runtimeGenerator.ts 92.91% <83.33%> (ø)
packages/ice/src/createService.ts 89.69% <100.00%> (ø)
packages/ice/src/service/webpackCompiler.ts 89.85% <100.00%> (ø)
packages/ice/src/types/plugin.ts 100.00% <100.00%> (ø)
packages/ice/src/webpack/DataLoaderPlugin.ts 94.94% <100.00%> (ø)
...ges/webpack-config/src/unPlugins/redirectImport.ts 81.86% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@chenjun1011 chenjun1011 changed the title [WIP] fix: redirect named import fix: redirect named import Mar 15, 2023
@chenjun1011 chenjun1011 added the need review Need Review label Mar 15, 2023
wssgcg1213
wssgcg1213 previously approved these changes Mar 16, 2023
Base automatically changed from release/next to master March 16, 2023 05:56
wssgcg1213
wssgcg1213 previously approved these changes Mar 16, 2023
luhc228
luhc228 previously approved these changes Mar 16, 2023
@ClarkXia ClarkXia changed the base branch from master to release/next March 16, 2023 07:17
@ClarkXia ClarkXia dismissed stale reviews from luhc228 and wssgcg1213 March 16, 2023 07:17

The base branch was changed.

@@ -3,8 +3,8 @@ import fse from 'fs-extra';
import type { Compiler } from 'webpack';
import webpack from '@ice/bundles/compiled/webpack/index.js';
import type { Context } from 'build-scripts';
import type { ServerCompiler, PluginData } from '../types/plugin.js';
import { IMPORT_META_RENDERER, IMPORT_META_TARGET, RUNTIME_TMP_DIR, RUNTIME_EXPORTS } from '../constant.js';
import type { ServerCompiler, PluginData, DeclarationData } from '../types/plugin.js';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

建议是从 packages/ice/src/types/generator.js 导入,DeclarationData 不属于 plugin

this.serverCompiler = serverCompiler;
this.rootDir = rootDir;
this.dataCache = dataCache;
this.getAllPlugin = getAllPlugin;
this.target = target;
this.exportList = exportList;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

建议在 DataLoaderPlugin 里 exportList 的命名可以显式说明是什么样的 exportList

按照

const exportList = generator.getExportList('framework', target);

可以考虑命名为 frameworkExportList

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

xxList 通常可以用复数形式表示

@chenjun1011 chenjun1011 requested a review from luhc228 March 17, 2023 05:59
@chenjun1011 chenjun1011 requested review from luhc228 and removed request for chenjun1011 March 17, 2023 08:00
@chenjun1011 chenjun1011 requested a review from answershuto March 17, 2023 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need review Need Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants