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

After pilet is packaged, export is added to the content of the module #570

Closed
wizard-a opened this issue Dec 22, 2022 · 12 comments · Fixed by #571
Closed

After pilet is packaged, export is added to the content of the module #570

wizard-a opened this issue Dec 22, 2022 · 12 comments · Fixed by #571
Labels
bug Something isn't working core Concerns the piral-core library. systemjs Module system and shared dependencies.
Milestone

Comments

@wizard-a
Copy link
Contributor

Bug Report

I wirte a pilet, imported some npm packages 。pilet 打包之后模块的内容会加上一些export

Look at the picture below, This is the content compiled by pilet

image

This is the content compiled by npm package。

企业微信截图_52b17613-671f-4dfb-9e5d-a42336f2bc8c

Why does pilet compile to change the contents of the npm package, which is a feature of system.js?

I looked at the piral-base code,Rewrite the System.constructor.prototype.register content 。
https://github.com/smapiot/piral/blob/HEAD/src/framework/piral-base/src/utils/system.ts#L66

After the code is wrapped by export, it will be executed to _export(...p);, The method does not return, causing the context to be lost
https://github.com/smapiot/piral/blob/HEAD/src/framework/piral-base/src/utils/system.ts#L93

Please tell me what to do , thanks !

@wizard-a wizard-a added the bug Something isn't working label Dec 22, 2022
@FlorianRappl
Copy link
Contributor

Sorry I do not understand what the problem is and why this is supposed to be a bug in Piral (rather than either your bundler or SystemJS).

The actual bundling is not done by Piral, but by a bundler like Webpack, esbuild, ... (whatever you choose here). The content that is in the pilet is fully processed by the bundler.

Can you also condense your problem into a MWE?

@FlorianRappl FlorianRappl added the repro-needed How to make it not work on my machine? label Dec 22, 2022
@wizard-a
Copy link
Contributor Author

look demo https://stackblitz.com/edit/vitejs-vite-3wa7no

Project structure

project/my-pilet pilet project

packages/test-pkg npm package

  1. In the pilet project, 'MyPage.tsx' file
 api.registerPage('/page', MyPage);
  1. The test-pkg package is import
import * as React from 'react';
import { SortableList } from 'test-pkg';

const Page1 = () => {
  return (
    <div>
      <div>page</div>
      <SortableList />
    </div>
  );
};

export default Page1;

  1. In the test-pkg package, File path page/Page.tsx
import React, { useMemo, useState } from 'react';

const DK = React.lazy(() => import('../layze/dk/DK'));
const DK1 = React.lazy(() => import('../layze/dk1/DK1'));

export const SortableList = () => {
  return (
    <>
      page1
      <DK />
      <DK1 />
    </>
  );
};


  1. In the path /layze/dk/DK file
import React, { useMemo, useState } from 'react';
import type { ReactNode } from 'react';
import { KeyboardCode } from '@dnd-kit/core';

import { ETest } from '../enum';

const test = [KeyboardCode.Down];

const DK = ({}) => {
  return (
    <>
      <h1>DK</h1>
      {ETest.name}
      {test[0]}
    </>
  );
};

export default DK;

===================================

  1. test-pkg build dist folder

image

  1. pilet build dist folder
    image

Why does pilet compile to change the contents of the npm package, which is a feature of system.js?

I looked at the piral-base code,Rewrite the System.constructor.prototype.register content 。
https://github.com/smapiot/piral/blob/HEAD/src/framework/piral-base/src/utils/system.ts#L66

After the code is wrapped by export, it will be executed to _export(...p);, The method does not return, causing the context to be lost
https://github.com/smapiot/piral/blob/HEAD/src/framework/piral-base/src/utils/system.ts#L93

@FlorianRappl
Copy link
Contributor

Well, I actually don't have any more information. These paragraphs

Why does pilet compile to change the contents of the npm package, which is a feature of system.js?

I looked at the piral-base code,Rewrite the System.constructor.prototype.register content 。
https://github.com/smapiot/piral/blob/HEAD/src/framework/piral-base/src/utils/system.ts#L66

After the code is wrapped by export, it will be executed to _export(...p);, The method does not return, causing the context to be lost
https://github.com/smapiot/piral/blob/HEAD/src/framework/piral-base/src/utils/system.ts#L93

don't make any sense at all. I am not sure what you are trying to tell me there (e.g., "pilet compile to change the contents of the npm package"; a bundled package will in general look different to its original source - but that does not mean that the behavior of the modules inside changed; it should be the same). The other two paragraphs are about runtime behavior when evaluating SystemJS modules, but have nothing to do with the introductory paragraph / problem description ("why does the content look different").

So all in all I still fail to understand the issue. What is the expectation? What is the behavior? Things are bundled, yes, but they are always bundled and if you don't want to bundle them then just write the pilet on your own. No need to use a bundler then.

Let's see if we find out what your core issue is, otherwise we'll need to close this one.

@wizard-a
Copy link
Contributor Author

I have no problem using the test-pkg package alone. After pilet compilation and system.js loading, the test-pkg package does not work properly.

You can visit demo and click the page menu。

image

Look at the stack information that is reporting an error

@FlorianRappl
Copy link
Contributor

Yeah but looking at it I'd rather say that is an issue with vite (it was never said in the reporting that the bundler used is vite - and that is a vital information here, so I wonder what else is missing from the report).

@wizard-a
Copy link
Contributor Author

I put a return in this code, and it works。

In this line

https://github.com/smapiot/piral/blob/HEAD/src/framework/piral-base/src/utils/system.ts#L93

return _export(...p);

@wizard-a
Copy link
Contributor Author

I'll try webpack

@FlorianRappl
Copy link
Contributor

I put a return in this code, and it works。

Yeah so if I understand you correctly the transformation to SystemJS assumes that the _exports inside will (probably only for calls with 2 arguments) return something and right now the wrapper does not. So explicitly returning this case solves the issue, right?

Mind doing a PR then? I wonder if this would need to be done for the 1-argument case, too, but it does not seem necessary.

@FlorianRappl FlorianRappl added core Concerns the piral-core library. systemjs Module system and shared dependencies. and removed repro-needed How to make it not work on my machine? labels Dec 22, 2022
@FlorianRappl FlorianRappl added this to the 0.15.5 milestone Dec 22, 2022
@wizard-a
Copy link
Contributor Author

I put a return in this code, and it works。

Yeah so if I understand you correctly the transformation to SystemJS assumes that the _exports inside will (probably only for calls with 2 arguments) return something and right now the wrapper does not. So explicitly returning this case solves the issue, right?

Mind doing a PR then? I wonder if this would need to be done for the 1-argument case, too, but it does not seem necessary.

Yes, Add return and the problem is solved 。
I propose a PR and hope to send out a version as soon as possible

@FlorianRappl
Copy link
Contributor

Awesome thanks!

@wizard-a
Copy link
Contributor Author

wizard-a commented Dec 23, 2022

@FlorianRappl
I desperately need to see if you can send a release

@FlorianRappl
Copy link
Contributor

It's already published using the next tag (https://www.npmjs.com/package/piral/v/0.15.5-beta.5022).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working core Concerns the piral-core library. systemjs Module system and shared dependencies.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants