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

Added assets version information #5299

Closed
wants to merge 1 commit into from

Conversation

core23
Copy link
Member

@core23 core23 commented Nov 14, 2018

I am targeting this branch, because this is pedantic.

Changelog

### Added
- Added `package.json` file for asset version information

Subject

This is the first step to remove external assets from this bundle.

@kunicmarko20
Copy link
Contributor

So what is the plan, is it outlined somewhere?

Roughly (correct me if I am wrong),

  • we add package.json
  • we remove all commited assets
  • we move our assets into an npm sonata-project/bundle-name-asset package?
  • we remove our assets?

@core23
Copy link
Member Author

core23 commented Nov 14, 2018

That's the idea.

My idea was to make the dependencies visible in the first step. I'm not that deep into the npm versioning, but can't we leave the php source and assets sources in the same repository and use git versioning for both?

@kunicmarko20
Copy link
Contributor

No idea, I guess we need someone that knows npm better than us

kunicmarko20
kunicmarko20 previously approved these changes Nov 14, 2018
greg0ire
greg0ire previously approved these changes Nov 15, 2018
@@ -0,0 +1,26 @@
{
"name": "@sonata-project/admin-bundle",
"version": "0.1.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why 0.1.0? Why not the current version of sonata-admin package?

Copy link
Member Author

Choose a reason for hiding this comment

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

We're little stuck on the asset problem. We can't use the current composer project version for the assets, because there we have no specific release process for the assets and they're not changing very often.

@osavchenko
Copy link
Contributor

Also, why not to remove bower from the project at all and configure npm packages properly? And why npm and not yarn, for example?

@core23
Copy link
Member Author

core23 commented Nov 28, 2018

Also, why not to remove bower from the project at all and configure npm packages properly?

Can you help with this?

And why npm and not yarn, for example?

IMHO yarn and npm are not that much different in handling package dependencies. They only use different technics for fetching them from the web.

@osavchenko
Copy link
Contributor

Can you help with this?

Yes, of course! But here is one kind of problem: npm doesn't support custom output directory. I think about this and have an idea to make postinstall script which moves node_modules directory to src/Resources/public/vendor. Is it sounds good?

@core23
Copy link
Member Author

core23 commented Nov 28, 2018

The idea is, that we don't have any vendor assets in our project. But no idea how to do this, as symfony doesn't have any good solution so far (symfony/webpack-encore#5)

@osavchenko
Copy link
Contributor

I think I need some advice. I found a way how to install deps in our vendor folder (with yarn install --modules-folder src/Resources/public/vendor, it can be wrapped with npm script command), but it installs deps of deps and it looks like:
image
And I think that this is not good. As an alternative, I have a suggestion to install gulp/grunt/webpack/etc to extract only that files that we really need for our admin and copy to vendor folder. In this case, it will be much clearer.
As a benefit of using gulp: a developer can extend his or her own gulpfile and process assets in another way as he or she wants. (don't know about other task managers).

@OskarStark
Copy link
Member

Why not using the Symfony Webpack way?

@core23
Copy link
Member Author

core23 commented Dec 20, 2018

Why not using the Symfony Webpack way?

There is no way to support asset dependencies of other bundles. You can only add assets for your projects. You don't know if a specific version of an asset (e.g. jQuery 2) will work with the bundle you require.

@core23 core23 requested a review from a team January 20, 2019 13:23
@core23
Copy link
Member Author

core23 commented Jan 20, 2019

Can we merge this?

@osavchenko
Copy link
Contributor

As for me it's useless in current state and will confuse developers who want to extend it somehow (bower/npm).

@core23
Copy link
Member Author

core23 commented Feb 3, 2019

Closing this in favor of #5461

@core23 core23 closed this Feb 3, 2019
@core23 core23 deleted the feature/npm branch February 3, 2019 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants