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: store error in vite HMR #4893

Merged
merged 8 commits into from
Nov 12, 2021
Merged

fix: store error in vite HMR #4893

merged 8 commits into from
Nov 12, 2021

Conversation

luhc228
Copy link
Collaborator

@luhc228 luhc228 commented Nov 10, 2021

问题

vite 项目下使用 store 会出现循环依赖的情况,从而导致 HMR 过程中发生报错:
can't read property store of null

@luhc228 luhc228 requested a review from ClarkXia November 10, 2021 08:45
@codecov-commenter
Copy link

codecov-commenter commented Nov 10, 2021

Codecov Report

Merging #4893 (fe112f6) into master (4a72e71) will decrease coverage by 0.07%.
The diff coverage is 20.37%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4893      +/-   ##
==========================================
- Coverage   25.46%   25.38%   -0.08%     
==========================================
  Files         192      193       +1     
  Lines        4835     4888      +53     
  Branches     1099     1109      +10     
==========================================
+ Hits         1231     1241      +10     
- Misses       3548     3591      +43     
  Partials       56       56              
Impacted Files Coverage Δ
...kages/plugin-store/src/vitePluginImportRedirect.ts 14.00% <14.00%> (ø)
packages/plugin-store/src/index.ts 82.85% <100.00%> (+0.76%) ⬆️

const pageStoreRegExp = /src\/pages\/\w+\/store.[jt]s$/;
if (appStoreRegExp.test(id) || pageStoreRegExp.test(id)) {
const s = new MagicString(code);
const matchedResult = code.match(/(createStore\s*,?).*} from ["']ice['"]/);
Copy link
Collaborator

Choose a reason for hiding this comment

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

正则 from 前后都可能出现不确定数量的空格

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

* import { Plugin } from 'ice';
* import { createStore } from '@ice/store';
*/
s.overwrite(matchedResult.index, matchedResult.index + matchedResult[1].length, '');
Copy link
Collaborator

Choose a reason for hiding this comment

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

单独 import { createStore } from 'ice'; 的场景结果是什么样的

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

变成 import { } from 'ice';

Copy link
Collaborator

Choose a reason for hiding this comment

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

这样不对吧

Copy link
Collaborator

Choose a reason for hiding this comment

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

单独 import { createStore } from 'ice'; 应该直接替换成 import { createStore } from '@ice/store';

@luhc228 luhc228 requested a review from ClarkXia November 10, 2021 09:05
const importDeclarations = getImportDeclarations();

return {
enforce: 'pre',
Copy link
Collaborator

Choose a reason for hiding this comment

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

用 es-module-lexer 的话就不能在 pre 阶段了

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

const [imports] = parse(code);
const s = new MagicString(code);
for (let index = 0; index < imports.length; index++) {
const importSpecifier = imports[0];
Copy link
Collaborator

Choose a reason for hiding this comment

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

imports[index]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

const normalImportSources: { [key: string]: string[] } = {};

importIdentifiersArray.forEach((importIdentifier: string) => {
const { value, type } = importDeclarations[importIdentifier];
Copy link
Collaborator

Choose a reason for hiding this comment

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

如果有一条不存在 理论上这个转化就没必要了,可以中断转化,并且输出一定的 warning 日志

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@ClarkXia ClarkXia requested a review from imsobear November 12, 2021 03:43
@luhc228 luhc228 merged commit d891e6e into master Nov 12, 2021
@luhc228 luhc228 deleted the fix/store-error-in-vite-HMR branch November 12, 2021 04:15
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

Successfully merging this pull request may close these issues.

4 participants